Re: [linux-sunxi] [PATCH v4 07/10] mfd: axp20x: add axp20x-regulator cell for AXP803

[icenowy]

在 2017-04-25 10:17，Chen-Yu Tsai 写道：
On Tue, Apr 25, 2017 at 12:01 AM, Icenowy Zheng  
wrote:

As axp20x-regulator now supports AXP803, add a cell for it.

Signed-off-by: Icenowy Zheng 
Acked-by: Chen-Yu Tsai 
---
Changes in v4:
- Added a trailing comma for new cell, for easier further cell 
addition.

Changes in v3:
- Make the new cell one-liner.

 drivers/mfd/axp20x.c | 3 ++-
 drivers/regulator/axp20x-regulator.c | 6 +++---


Squashed in wrong patch?


Oh seems so...



ChenYu

Re: [linux-sunxi] [PATCH v4 07/10] mfd: axp20x: add axp20x-regulator cell for AXP803

[icenowy]

在 2017-04-25 10:17，Chen-Yu Tsai 写道：
On Tue, Apr 25, 2017 at 12:01 AM, Icenowy Zheng  
wrote:

As axp20x-regulator now supports AXP803, add a cell for it.

Signed-off-by: Icenowy Zheng 
Acked-by: Chen-Yu Tsai 
---
Changes in v4:
- Added a trailing comma for new cell, for easier further cell 
addition.

Changes in v3:
- Make the new cell one-liner.

 drivers/mfd/axp20x.c | 3 ++-
 drivers/regulator/axp20x-regulator.c | 6 +++---


Squashed in wrong patch?


Oh seems so...



ChenYu

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

On 2017-04-24 23:27, Andy Shevchenko wrote:
> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka  wrote:
>> The IOT2000 is industrial controller platform, derived from the Intel
>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>> IOT2040 has two of them. They can be told apart based on the board asset
>> tag in the DMI table.
>>
>> Based on patch by Sascha Weisenberger.
>>
> 
>> Signed-off-by: Jan Kiszka 
>> Signed-off-by: Sascha Weisenberger 
> 
> Shoudn't be ordered other way around?

Nope. My changes invalidated Sascha's signed-off on the original patch,
but he signed off again on the final version.

> 
>> +   const char *asset_tag;
> 
> I guess this is redundant. See below.
> 
>> +   {
>> +   .name = "SIMATIC IOT2000",
>> +   .asset_tag = "6ES7647-0AA00-0YA2",
>> +   .func = 6,
>> +   .phy_addr = 1,
>> +   },
> 
> The below has same definition disregard on asset_tag.
> 

There is a small difference in the asset tag, just not at the last digit
where one may expect it, look:

...-0YA2 -> IOT2020
...-1YA2 -> IOT2040

>> +   {
>> +   .name = "SIMATIC IOT2000",
>> +   .asset_tag = "6ES7647-0AA00-1YA2",
>> +   .func = 6,
>> +   .phy_addr = 1,
>> +   },
>> +   {
>> +   .name = "SIMATIC IOT2000",
>> +   .asset_tag = "6ES7647-0AA00-1YA2",
>> +   .func = 7,
>> +   .phy_addr = 1,
>> +   },
> 
> How this supposed to work if phy_addr is the same?
> 

That address space is MAC-local, and we have two different MACs here.

Jan

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

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

On 2017-04-24 23:27, Andy Shevchenko wrote:
> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka  wrote:
>> The IOT2000 is industrial controller platform, derived from the Intel
>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>> IOT2040 has two of them. They can be told apart based on the board asset
>> tag in the DMI table.
>>
>> Based on patch by Sascha Weisenberger.
>>
> 
>> Signed-off-by: Jan Kiszka 
>> Signed-off-by: Sascha Weisenberger 
> 
> Shoudn't be ordered other way around?

Nope. My changes invalidated Sascha's signed-off on the original patch,
but he signed off again on the final version.

> 
>> +   const char *asset_tag;
> 
> I guess this is redundant. See below.
> 
>> +   {
>> +   .name = "SIMATIC IOT2000",
>> +   .asset_tag = "6ES7647-0AA00-0YA2",
>> +   .func = 6,
>> +   .phy_addr = 1,
>> +   },
> 
> The below has same definition disregard on asset_tag.
> 

There is a small difference in the asset tag, just not at the last digit
where one may expect it, look:

...-0YA2 -> IOT2020
...-1YA2 -> IOT2040

>> +   {
>> +   .name = "SIMATIC IOT2000",
>> +   .asset_tag = "6ES7647-0AA00-1YA2",
>> +   .func = 6,
>> +   .phy_addr = 1,
>> +   },
>> +   {
>> +   .name = "SIMATIC IOT2000",
>> +   .asset_tag = "6ES7647-0AA00-1YA2",
>> +   .func = 7,
>> +   .phy_addr = 1,
>> +   },
> 
> How this supposed to work if phy_addr is the same?
> 

That address space is MAC-local, and we have two different MACs here.

Jan

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

Re: [PATCH] iio: adc: Add support for TI ADC1x8s102

[Jan Kiszka]

On 2017-04-24 23:25, Andy Shevchenko wrote:
> On Mon, Apr 24, 2017 at 11:32 PM, Jan Kiszka  wrote:
>> On 2017-04-24 22:05, Andy Shevchenko wrote:
>>> On Mon, 2017-04-24 at 21:28 +0200, Jan Kiszka wrote:
 This is an upstream port of an IIO driver for the TI ADC108S102 and
 ADC128S102. The former can be found on the Intel Galileo Gen2 and the
 Siemens SIMATIC IOT2000. For those boards, ACPI-based enumeration is
 included.
> 
 +#ifdef CONFIG_ACPI
 +typedef int (*acpi_setup_handler)(struct spi_device *,
 +  const struct
 adc1x8s102_platform_data **);
 +
 +static const struct adc1x8s102_platform_data int3495_platform_data =
 {
 +.ext_vin = 5000,/* 5 V */
 +};
 +
>>>
 +/* Galileo Gen 2 SPI setup */
 +static int
 +adc1x8s102_setup_int3495(struct spi_device *spi,
 + const struct adc1x8s102_platform_data
 **pdata)
 +{
>>>
 +struct pxa2xx_spi_chip *chip_data;
>>>
>>> This one is too big to waste memory on one member.
>>>
 +
 +chip_data = devm_kzalloc(>dev, sizeof(*chip_data),
 GFP_KERNEL);
 +if (!chip_data)
 +return -ENOMEM;
 +
 +chip_data->gpio_cs = ADC1x8S102_GALILEO2_CS;
 +spi->controller_data = chip_data;
 +dev_info(>dev, "setting GPIO CS value to %d\n",
 + chip_data->gpio_cs);
 +spi_setup(spi);
 +
 +*pdata = _platform_data;
 +
 +return 0;
 +}
>>>
>>> This is weird approach.
>>
>> Let me dig deeper if are allowed to pass a static struct here as well.
>> But the struct is driver-defined.
> 
> We have _DSD for ACPI, that's why I sent another email where I was
> asking for DSDT excerpt and if it's already in the wild.

I don't find any traces of "_DSD" in those DSDTs.

> 
>>
>>> Moreover, please do not use platform data at all.
>>
>> That is just following pre-existing pattern, just look around in the
>> iio/adc folder, not to speak of others. But I'm open to learn about any
>> newer pattern there is.
> 
> Unified Device Properties API is your friend. It makes driver to
> consume resources in agnostic way.

Is that ACPI-only or a generic solution? Where is a good example? Sorry,
I still don't see how to make code out of your comments.

> 
 +static const struct acpi_device_id adc1x8s102_acpi_ids[] = {
 +{ "INT3495",  (kernel_ulong_t)_setup_int3495 },
 +{ }
 +};
 +MODULE_DEVICE_TABLE(acpi, adc1x8s102_acpi_ids);
 +#endif
 +
 +static int adc1x8s102_probe(struct spi_device *spi)
 +{
 +const struct adc1x8s102_platform_data *pdata = spi-
> dev.platform_data;
 +struct adc1x8s102_state *st;
 +struct iio_dev *indio_dev;
 +int ret;
 +
>>>
 +#ifdef CONFIG_ACPI
>>>
>>> No.
>>
>> ...because?
> 
> Because in correctly written ->probe() all ACPI functions have stubs
> for !CONFIG_ACPI case. Just no need.

OK, will give that a try. I just don't want to leave much dead code
behind for !CONFIG_ACPI.

> 
 +setup_handler = (acpi_setup_handler)id->driver_data;
 +if (setup_handler) {
 +ret = setup_handler(spi, );
 +if (ret)
 +return ret;
 +}
>>>
>>> No way.
>>
>> Constructive feedback, please.
> 
> See above. We have nowadays mechanisms to provide device properties natively.
> Without seeing DSDT I can't tell more.

You've seen it, please tell me more now.

> 
 +++ b/include/linux/platform_data/adc1x8s102.h
>>>
>>> It must be no such file at all!
>>> Please, remove it completely.
>>
>> Not without explaining what the new style is. As I said, the existing
>> driver use that as well.
> 
> See above.
> 
>> The fact that there is no OF binding yet
>> exploiting this should be no excuse IMHO.
> 
> ...and I'm not talking about it at all.
> 

But I am: ACPI is not the center of the world (luckily), and this driver
shall not be designed to only work with that way of defining resources.
Therefore, I'm trying to follow driver which include OF support.

Jan

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

Re: [PATCH] iio: adc: Add support for TI ADC1x8s102

[Jan Kiszka]

On 2017-04-24 23:25, Andy Shevchenko wrote:
> On Mon, Apr 24, 2017 at 11:32 PM, Jan Kiszka  wrote:
>> On 2017-04-24 22:05, Andy Shevchenko wrote:
>>> On Mon, 2017-04-24 at 21:28 +0200, Jan Kiszka wrote:
 This is an upstream port of an IIO driver for the TI ADC108S102 and
 ADC128S102. The former can be found on the Intel Galileo Gen2 and the
 Siemens SIMATIC IOT2000. For those boards, ACPI-based enumeration is
 included.
> 
 +#ifdef CONFIG_ACPI
 +typedef int (*acpi_setup_handler)(struct spi_device *,
 +  const struct
 adc1x8s102_platform_data **);
 +
 +static const struct adc1x8s102_platform_data int3495_platform_data =
 {
 +.ext_vin = 5000,/* 5 V */
 +};
 +
>>>
 +/* Galileo Gen 2 SPI setup */
 +static int
 +adc1x8s102_setup_int3495(struct spi_device *spi,
 + const struct adc1x8s102_platform_data
 **pdata)
 +{
>>>
 +struct pxa2xx_spi_chip *chip_data;
>>>
>>> This one is too big to waste memory on one member.
>>>
 +
 +chip_data = devm_kzalloc(>dev, sizeof(*chip_data),
 GFP_KERNEL);
 +if (!chip_data)
 +return -ENOMEM;
 +
 +chip_data->gpio_cs = ADC1x8S102_GALILEO2_CS;
 +spi->controller_data = chip_data;
 +dev_info(>dev, "setting GPIO CS value to %d\n",
 + chip_data->gpio_cs);
 +spi_setup(spi);
 +
 +*pdata = _platform_data;
 +
 +return 0;
 +}
>>>
>>> This is weird approach.
>>
>> Let me dig deeper if are allowed to pass a static struct here as well.
>> But the struct is driver-defined.
> 
> We have _DSD for ACPI, that's why I sent another email where I was
> asking for DSDT excerpt and if it's already in the wild.

I don't find any traces of "_DSD" in those DSDTs.

> 
>>
>>> Moreover, please do not use platform data at all.
>>
>> That is just following pre-existing pattern, just look around in the
>> iio/adc folder, not to speak of others. But I'm open to learn about any
>> newer pattern there is.
> 
> Unified Device Properties API is your friend. It makes driver to
> consume resources in agnostic way.

Is that ACPI-only or a generic solution? Where is a good example? Sorry,
I still don't see how to make code out of your comments.

> 
 +static const struct acpi_device_id adc1x8s102_acpi_ids[] = {
 +{ "INT3495",  (kernel_ulong_t)_setup_int3495 },
 +{ }
 +};
 +MODULE_DEVICE_TABLE(acpi, adc1x8s102_acpi_ids);
 +#endif
 +
 +static int adc1x8s102_probe(struct spi_device *spi)
 +{
 +const struct adc1x8s102_platform_data *pdata = spi-
> dev.platform_data;
 +struct adc1x8s102_state *st;
 +struct iio_dev *indio_dev;
 +int ret;
 +
>>>
 +#ifdef CONFIG_ACPI
>>>
>>> No.
>>
>> ...because?
> 
> Because in correctly written ->probe() all ACPI functions have stubs
> for !CONFIG_ACPI case. Just no need.

OK, will give that a try. I just don't want to leave much dead code
behind for !CONFIG_ACPI.

> 
 +setup_handler = (acpi_setup_handler)id->driver_data;
 +if (setup_handler) {
 +ret = setup_handler(spi, );
 +if (ret)
 +return ret;
 +}
>>>
>>> No way.
>>
>> Constructive feedback, please.
> 
> See above. We have nowadays mechanisms to provide device properties natively.
> Without seeing DSDT I can't tell more.

You've seen it, please tell me more now.

> 
 +++ b/include/linux/platform_data/adc1x8s102.h
>>>
>>> It must be no such file at all!
>>> Please, remove it completely.
>>
>> Not without explaining what the new style is. As I said, the existing
>> driver use that as well.
> 
> See above.
> 
>> The fact that there is no OF binding yet
>> exploiting this should be no excuse IMHO.
> 
> ...and I'm not talking about it at all.
> 

But I am: ACPI is not the center of the world (luckily), and this driver
shall not be designed to only work with that way of defining resources.
Therefore, I'm trying to follow driver which include OF support.

Jan

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

Re: [PATCH v6 05/15] lockdep: Implement crossrelease feature

[Byungchul Park]

On Mon, Apr 24, 2017 at 12:17:47PM +0200, Peter Zijlstra wrote:
> On Mon, Apr 24, 2017 at 02:11:02PM +0900, Byungchul Park wrote:
> > On Wed, Apr 19, 2017 at 04:25:03PM +0200, Peter Zijlstra wrote:
> 
> > > I still don't like work_id; it doesn't have anything to do with
> > > workqueues per se, other than the fact that they end up using it.
> > > 
> > > It's a history generation id; touching it completely invalidates our
> > > history. Workqueues need this because they run independent work from the
> > > same context.
> > > 
> > > But the same is true for other sites. Last time I suggested
> > > lockdep_assert_empty() to denote all suck places (and note we already
> > > have lockdep_sys_exit() that hooks into the return to user path).
> > 
> > I'm sorry but I don't understand what you intend. It would be appriciated
> > if you explain more.
> > 
> > You might know why I introduced the 'work_id'.. Is there any alternative?
> 
> My complaint is mostly about naming.. and "hist_gen_id" might be a
> better name.

Ah, I also think the name, 'work_id', is not good... and frankly I am
not sure if 'hist_gen_id' is good, either. What about to apply 'rollback',
which I did for locks in irq, into works of workqueues? If you say yes,
I will try to do it.

> But let me explain.
> 
> 
> The reason workqueues need this is because the lock history for each
> 'work' are independent. The locks of Work-B do not depend on the locks
> of the preceding Work-A, because the completion of Work-B is not
> dependent on those locks.
> 
> But this is true for many things; pretty much all kthreads fall in this
> pattern, where they have an 'idle' state and future completions do not
> depend on past completions. Its just that since they all have the 'same'
> form -- the kthread does the same over and over -- it doesn't matter
> much.
> 
> The same is true for system-calls, once a system call is complete (we've
> returned to userspace) the next system call does not depend on the lock
> history of the previous one.

Yes. I agree. As you said, actually two independent job e.g. syscalls,
works.. should not depend on each other.

Frankly speaking, nevertheless, if they depend on each other, then I
think it would be better to detect the cases, too. But for now, since
it's more important to avoid false positive detections, I will do it as
conservatively as possible, as my current implementation.

And thank you for additional explanation!

Re: [PATCH v6 05/15] lockdep: Implement crossrelease feature

[Byungchul Park]

On Mon, Apr 24, 2017 at 12:17:47PM +0200, Peter Zijlstra wrote:
> On Mon, Apr 24, 2017 at 02:11:02PM +0900, Byungchul Park wrote:
> > On Wed, Apr 19, 2017 at 04:25:03PM +0200, Peter Zijlstra wrote:
> 
> > > I still don't like work_id; it doesn't have anything to do with
> > > workqueues per se, other than the fact that they end up using it.
> > > 
> > > It's a history generation id; touching it completely invalidates our
> > > history. Workqueues need this because they run independent work from the
> > > same context.
> > > 
> > > But the same is true for other sites. Last time I suggested
> > > lockdep_assert_empty() to denote all suck places (and note we already
> > > have lockdep_sys_exit() that hooks into the return to user path).
> > 
> > I'm sorry but I don't understand what you intend. It would be appriciated
> > if you explain more.
> > 
> > You might know why I introduced the 'work_id'.. Is there any alternative?
> 
> My complaint is mostly about naming.. and "hist_gen_id" might be a
> better name.

Ah, I also think the name, 'work_id', is not good... and frankly I am
not sure if 'hist_gen_id' is good, either. What about to apply 'rollback',
which I did for locks in irq, into works of workqueues? If you say yes,
I will try to do it.

> But let me explain.
> 
> 
> The reason workqueues need this is because the lock history for each
> 'work' are independent. The locks of Work-B do not depend on the locks
> of the preceding Work-A, because the completion of Work-B is not
> dependent on those locks.
> 
> But this is true for many things; pretty much all kthreads fall in this
> pattern, where they have an 'idle' state and future completions do not
> depend on past completions. Its just that since they all have the 'same'
> form -- the kthread does the same over and over -- it doesn't matter
> much.
> 
> The same is true for system-calls, once a system call is complete (we've
> returned to userspace) the next system call does not depend on the lock
> history of the previous one.

Yes. I agree. As you said, actually two independent job e.g. syscalls,
works.. should not depend on each other.

Frankly speaking, nevertheless, if they depend on each other, then I
think it would be better to detect the cases, too. But for now, since
it's more important to avoid false positive detections, I will do it as
conservatively as possible, as my current implementation.

And thank you for additional explanation!

Re: [PATCH RT] usb: Use _nort in usb_hcd_pci_remove

[kbuild test robot]

Hi Nate,

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.11-rc8 next-20170424]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Nate-Dailey/usb-Use-_nort-in-usb_hcd_pci_remove/20170424-214548
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
usb-testing
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/usb//core/hcd-pci.c: In function 'usb_hcd_pci_remove':
>> drivers/usb//core/hcd-pci.c:344:2: error: implicit declaration of function 
>> 'local_irq_disable_nort' [-Werror=implicit-function-declaration]
 local_irq_disable_nort();
 ^~
>> drivers/usb//core/hcd-pci.c:346:2: error: implicit declaration of function 
>> 'local_irq_enable_nort' [-Werror=implicit-function-declaration]
 local_irq_enable_nort();
 ^
   cc1: some warnings being treated as errors

vim +/local_irq_disable_nort +344 drivers/usb//core/hcd-pci.c

   338  pm_runtime_get_noresume(>dev);
   339  
   340  /* Fake an interrupt request in order to give the driver a 
chance
   341   * to test whether the controller hardware has been removed 
(e.g.,
   342   * cardbus physical eject).
   343   */
 > 344  local_irq_disable_nort();
   345  usb_hcd_irq(0, hcd);
 > 346  local_irq_enable_nort();
   347  
   348  /* Note: dev_set_drvdata must be called while holding the rwsem 
*/
   349  if (dev->class == CL_EHCI) {

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH RT] usb: Use _nort in usb_hcd_pci_remove

[kbuild test robot]

Hi Nate,

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.11-rc8 next-20170424]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Nate-Dailey/usb-Use-_nort-in-usb_hcd_pci_remove/20170424-214548
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
usb-testing
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/usb//core/hcd-pci.c: In function 'usb_hcd_pci_remove':
>> drivers/usb//core/hcd-pci.c:344:2: error: implicit declaration of function 
>> 'local_irq_disable_nort' [-Werror=implicit-function-declaration]
 local_irq_disable_nort();
 ^~
>> drivers/usb//core/hcd-pci.c:346:2: error: implicit declaration of function 
>> 'local_irq_enable_nort' [-Werror=implicit-function-declaration]
 local_irq_enable_nort();
 ^
   cc1: some warnings being treated as errors

vim +/local_irq_disable_nort +344 drivers/usb//core/hcd-pci.c

   338  pm_runtime_get_noresume(>dev);
   339  
   340  /* Fake an interrupt request in order to give the driver a 
chance
   341   * to test whether the controller hardware has been removed 
(e.g.,
   342   * cardbus physical eject).
   343   */
 > 344  local_irq_disable_nort();
   345  usb_hcd_irq(0, hcd);
 > 346  local_irq_enable_nort();
   347  
   348  /* Note: dev_set_drvdata must be called while holding the rwsem 
*/
   349  if (dev->class == CL_EHCI) {

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v3] usb: typec: Don't prevent using constant typec_mode_desc initializers

[Guenter Roeck]

On Mon, Apr 24, 2017 at 11:52 AM, Mats Karrman  wrote:
> Signed-off-by: Mats Karrman 
> ---
> v3:
> - Fixed damaged white-space, finally?
>

Yes, this one finally applied. Some description in addition to the
subject line might be nice. Other than that,

Reviewed-by: Guenter Roeck 

Guenter

> v2:
> - Fixed damaged white-space
> - Added signed-off-by
>
>  drivers/usb/typec/typec.c | 11 ++-
>  include/linux/usb/typec.h |  6 +++---
>  2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
> index 89e540b..db5ee73 100644
> --- a/drivers/usb/typec/typec.c
> +++ b/drivers/usb/typec/typec.c
> @@ -291,7 +291,7 @@ typec_altmode_roles_show(struct device *dev, struct 
> device_attribute *attr,
>  }
>
>  static void typec_init_modes(struct typec_altmode *alt,
> -struct typec_mode_desc *desc, bool is_port)
> +const struct typec_mode_desc *desc, bool is_port)
>  {
> int i;
>
> @@ -378,7 +378,8 @@ static const struct device_type typec_altmode_dev_type = {
>  };
>
>  static struct typec_altmode *
> -typec_register_altmode(struct device *parent, struct typec_altmode_desc 
> *desc)
> +typec_register_altmode(struct device *parent,
> +  const struct typec_altmode_desc *desc)
>  {
> struct typec_altmode *alt;
> int ret;
> @@ -495,7 +496,7 @@ EXPORT_SYMBOL_GPL(typec_partner_set_identity);
>   */
>  struct typec_altmode *
>  typec_partner_register_altmode(struct typec_partner *partner,
> -  struct typec_altmode_desc *desc)
> +  const struct typec_altmode_desc *desc)
>  {
> return typec_register_altmode(>dev, desc);
>  }
> @@ -590,7 +591,7 @@ static const struct device_type typec_plug_dev_type = {
>   */
>  struct typec_altmode *
>  typec_plug_register_altmode(struct typec_plug *plug,
> -   struct typec_altmode_desc *desc)
> +   const struct typec_altmode_desc *desc)
>  {
> return typec_register_altmode(>dev, desc);
>  }
> @@ -1159,7 +1160,7 @@ EXPORT_SYMBOL_GPL(typec_set_pwr_opmode);
>   */
>  struct typec_altmode *
>  typec_port_register_altmode(struct typec_port *port,
> -   struct typec_altmode_desc *desc)
> +   const struct typec_altmode_desc *desc)
>  {
> return typec_register_altmode(>dev, desc);
>  }
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index ec78204..d1d2ebc 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -117,13 +117,13 @@ struct typec_altmode_desc {
>
>  struct typec_altmode
>  *typec_partner_register_altmode(struct typec_partner *partner,
> -   struct typec_altmode_desc *desc);
> +   const struct typec_altmode_desc *desc);
>  struct typec_altmode
>  *typec_plug_register_altmode(struct typec_plug *plug,
> -struct typec_altmode_desc *desc);
> +const struct typec_altmode_desc *desc);
>  struct typec_altmode
>  *typec_port_register_altmode(struct typec_port *port,
> -struct typec_altmode_desc *desc);
> +const struct typec_altmode_desc *desc);
>  void typec_unregister_altmode(struct typec_altmode *altmode);
>
>  struct typec_port *typec_altmode2port(struct typec_altmode *alt);
> --
> 2.1.4
>

Re: [PATCH v3] usb: typec: Don't prevent using constant typec_mode_desc initializers

[Guenter Roeck]

On Mon, Apr 24, 2017 at 11:52 AM, Mats Karrman  wrote:
> Signed-off-by: Mats Karrman 
> ---
> v3:
> - Fixed damaged white-space, finally?
>

Yes, this one finally applied. Some description in addition to the
subject line might be nice. Other than that,

Reviewed-by: Guenter Roeck 

Guenter

> v2:
> - Fixed damaged white-space
> - Added signed-off-by
>
>  drivers/usb/typec/typec.c | 11 ++-
>  include/linux/usb/typec.h |  6 +++---
>  2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
> index 89e540b..db5ee73 100644
> --- a/drivers/usb/typec/typec.c
> +++ b/drivers/usb/typec/typec.c
> @@ -291,7 +291,7 @@ typec_altmode_roles_show(struct device *dev, struct 
> device_attribute *attr,
>  }
>
>  static void typec_init_modes(struct typec_altmode *alt,
> -struct typec_mode_desc *desc, bool is_port)
> +const struct typec_mode_desc *desc, bool is_port)
>  {
> int i;
>
> @@ -378,7 +378,8 @@ static const struct device_type typec_altmode_dev_type = {
>  };
>
>  static struct typec_altmode *
> -typec_register_altmode(struct device *parent, struct typec_altmode_desc 
> *desc)
> +typec_register_altmode(struct device *parent,
> +  const struct typec_altmode_desc *desc)
>  {
> struct typec_altmode *alt;
> int ret;
> @@ -495,7 +496,7 @@ EXPORT_SYMBOL_GPL(typec_partner_set_identity);
>   */
>  struct typec_altmode *
>  typec_partner_register_altmode(struct typec_partner *partner,
> -  struct typec_altmode_desc *desc)
> +  const struct typec_altmode_desc *desc)
>  {
> return typec_register_altmode(>dev, desc);
>  }
> @@ -590,7 +591,7 @@ static const struct device_type typec_plug_dev_type = {
>   */
>  struct typec_altmode *
>  typec_plug_register_altmode(struct typec_plug *plug,
> -   struct typec_altmode_desc *desc)
> +   const struct typec_altmode_desc *desc)
>  {
> return typec_register_altmode(>dev, desc);
>  }
> @@ -1159,7 +1160,7 @@ EXPORT_SYMBOL_GPL(typec_set_pwr_opmode);
>   */
>  struct typec_altmode *
>  typec_port_register_altmode(struct typec_port *port,
> -   struct typec_altmode_desc *desc)
> +   const struct typec_altmode_desc *desc)
>  {
> return typec_register_altmode(>dev, desc);
>  }
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index ec78204..d1d2ebc 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -117,13 +117,13 @@ struct typec_altmode_desc {
>
>  struct typec_altmode
>  *typec_partner_register_altmode(struct typec_partner *partner,
> -   struct typec_altmode_desc *desc);
> +   const struct typec_altmode_desc *desc);
>  struct typec_altmode
>  *typec_plug_register_altmode(struct typec_plug *plug,
> -struct typec_altmode_desc *desc);
> +const struct typec_altmode_desc *desc);
>  struct typec_altmode
>  *typec_port_register_altmode(struct typec_port *port,
> -struct typec_altmode_desc *desc);
> +const struct typec_altmode_desc *desc);
>  void typec_unregister_altmode(struct typec_altmode *altmode);
>
>  struct typec_port *typec_altmode2port(struct typec_altmode *alt);
> --
> 2.1.4
>

Re: [PATCH] xen/scsifront: use offset_in_page() macro

[Juergen Gross]

On 25/04/17 00:15, Martin K. Petersen wrote:
> 
> Juergen,
> 
>> On 22/04/17 03:21, Geliang Tang wrote:
>>> Use offset_in_page() macro instead of open-coding.
>>>
>>> Signed-off-by: Geliang Tang 
>>
>> Reviewed-by: Juergen Gross 
> 
> Taking this through the Xen tree or should I queue it?

I can take it through the Xen tree.


Thanks,

Juergen

Re: [PATCH] xen/scsifront: use offset_in_page() macro

[Juergen Gross]

On 25/04/17 00:15, Martin K. Petersen wrote:
> 
> Juergen,
> 
>> On 22/04/17 03:21, Geliang Tang wrote:
>>> Use offset_in_page() macro instead of open-coding.
>>>
>>> Signed-off-by: Geliang Tang 
>>
>> Reviewed-by: Juergen Gross 
> 
> Taking this through the Xen tree or should I queue it?

I can take it through the Xen tree.


Thanks,

Juergen

Re: net/ipv6: slab-out-of-bounds in ip6_tnl_xmit

[Cong Wang]

On Mon, Apr 24, 2017 at 9:47 AM, Cong Wang  wrote:
>
> We use ipv4 dst in ip6_tunnel and cast an IPv4 neigh key as an
> IPv6 address...
>
>
> neigh = dst_neigh_lookup(skb_dst(skb),
>  _hdr(skb)->daddr);
> if (!neigh)
> goto tx_err_link_failure;
>
> addr6 = (struct in6_addr *)>primary_key; // <=== HERE
> addr_type = ipv6_addr_type(addr6);
>
> if (addr_type == IPV6_ADDR_ANY)
> addr6 = _hdr(skb)->daddr;
>
> memcpy(>daddr, addr6, sizeof(fl6->daddr));
>
> Also the network header of the skb at this point should be still IPv4?

Please try the attached patch.

I am not sure how we could handle 4in6 case better than just relying on
the config of ip6 tunnel.
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 75fac93..a9692ec 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1037,7 +1037,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device 
*dev, __u8 dsfield,
struct ip6_tnl *t = netdev_priv(dev);
struct net *net = t->net;
struct net_device_stats *stats = >dev->stats;
-   struct ipv6hdr *ipv6h = ipv6_hdr(skb);
+   struct ipv6hdr *ipv6h;
struct ipv6_tel_txoption opt;
struct dst_entry *dst = NULL, *ndst = NULL;
struct net_device *tdev;
@@ -1057,26 +1057,28 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device 
*dev, __u8 dsfield,
 
/* NBMA tunnel */
if (ipv6_addr_any(>parms.raddr)) {
-   struct in6_addr *addr6;
-   struct neighbour *neigh;
-   int addr_type;
+   if (skb->protocol == htons(ETH_P_IPV6)) {
+   struct in6_addr *addr6;
+   struct neighbour *neigh;
+   int addr_type;
 
-   if (!skb_dst(skb))
-   goto tx_err_link_failure;
+   if (!skb_dst(skb))
+   goto tx_err_link_failure;
 
-   neigh = dst_neigh_lookup(skb_dst(skb),
-_hdr(skb)->daddr);
-   if (!neigh)
-   goto tx_err_link_failure;
+   neigh = dst_neigh_lookup(skb_dst(skb),
+_hdr(skb)->daddr);
+   if (!neigh)
+   goto tx_err_link_failure;
 
-   addr6 = (struct in6_addr *)>primary_key;
-   addr_type = ipv6_addr_type(addr6);
+   addr6 = (struct in6_addr *)>primary_key;
+   addr_type = ipv6_addr_type(addr6);
 
-   if (addr_type == IPV6_ADDR_ANY)
-   addr6 = _hdr(skb)->daddr;
+   if (addr_type == IPV6_ADDR_ANY)
+   addr6 = _hdr(skb)->daddr;
 
-   memcpy(>daddr, addr6, sizeof(fl6->daddr));
-   neigh_release(neigh);
+   memcpy(>daddr, addr6, sizeof(fl6->daddr));
+   neigh_release(neigh);
+   }
} else if (!(t->parms.flags &
 (IP6_TNL_F_USE_ORIG_TCLASS | IP6_TNL_F_USE_ORIG_FWMARK))) {
/* enable the cache only only if the routing decision does

Re: net/ipv6: slab-out-of-bounds in ip6_tnl_xmit

[Cong Wang]

On Mon, Apr 24, 2017 at 9:47 AM, Cong Wang  wrote:
>
> We use ipv4 dst in ip6_tunnel and cast an IPv4 neigh key as an
> IPv6 address...
>
>
> neigh = dst_neigh_lookup(skb_dst(skb),
>  _hdr(skb)->daddr);
> if (!neigh)
> goto tx_err_link_failure;
>
> addr6 = (struct in6_addr *)>primary_key; // <=== HERE
> addr_type = ipv6_addr_type(addr6);
>
> if (addr_type == IPV6_ADDR_ANY)
> addr6 = _hdr(skb)->daddr;
>
> memcpy(>daddr, addr6, sizeof(fl6->daddr));
>
> Also the network header of the skb at this point should be still IPv4?

Please try the attached patch.

I am not sure how we could handle 4in6 case better than just relying on
the config of ip6 tunnel.
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 75fac93..a9692ec 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1037,7 +1037,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device 
*dev, __u8 dsfield,
struct ip6_tnl *t = netdev_priv(dev);
struct net *net = t->net;
struct net_device_stats *stats = >dev->stats;
-   struct ipv6hdr *ipv6h = ipv6_hdr(skb);
+   struct ipv6hdr *ipv6h;
struct ipv6_tel_txoption opt;
struct dst_entry *dst = NULL, *ndst = NULL;
struct net_device *tdev;
@@ -1057,26 +1057,28 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device 
*dev, __u8 dsfield,
 
/* NBMA tunnel */
if (ipv6_addr_any(>parms.raddr)) {
-   struct in6_addr *addr6;
-   struct neighbour *neigh;
-   int addr_type;
+   if (skb->protocol == htons(ETH_P_IPV6)) {
+   struct in6_addr *addr6;
+   struct neighbour *neigh;
+   int addr_type;
 
-   if (!skb_dst(skb))
-   goto tx_err_link_failure;
+   if (!skb_dst(skb))
+   goto tx_err_link_failure;
 
-   neigh = dst_neigh_lookup(skb_dst(skb),
-_hdr(skb)->daddr);
-   if (!neigh)
-   goto tx_err_link_failure;
+   neigh = dst_neigh_lookup(skb_dst(skb),
+_hdr(skb)->daddr);
+   if (!neigh)
+   goto tx_err_link_failure;
 
-   addr6 = (struct in6_addr *)>primary_key;
-   addr_type = ipv6_addr_type(addr6);
+   addr6 = (struct in6_addr *)>primary_key;
+   addr_type = ipv6_addr_type(addr6);
 
-   if (addr_type == IPV6_ADDR_ANY)
-   addr6 = _hdr(skb)->daddr;
+   if (addr_type == IPV6_ADDR_ANY)
+   addr6 = _hdr(skb)->daddr;
 
-   memcpy(>daddr, addr6, sizeof(fl6->daddr));
-   neigh_release(neigh);
+   memcpy(>daddr, addr6, sizeof(fl6->daddr));
+   neigh_release(neigh);
+   }
} else if (!(t->parms.flags &
 (IP6_TNL_F_USE_ORIG_TCLASS | IP6_TNL_F_USE_ORIG_FWMARK))) {
/* enable the cache only only if the routing decision does

Re: [PATCH 0/2] DS1374 Watchdog fixes

[Guenter Roeck]

On 04/24/2017 03:05 PM, Moritz Fischer wrote:

Hi all,

this series fixes two issues that I ran into when trying to use the
watchdog part of the DS1374 on of our products.

This series is basically a precursor to some further cleanup work,
that turns the DS1374 into a MFD device with an RTC and WDT in
separate drivers [1] each integrated in either the watchdog and
the rtc frameworks.

I'm very unhappy with the CONFIG_DRV_RTC_DS1374_WDT way of enabling
the watchdog behavior and currently I'm investigating how to make
that work via DT.

Watchdog maintainers, do you have an idea on how to do that in a
non breaking fashion?



Depends on what you mean with "non breaking". Just using the normal mfd
mechanisms, ie define an mfd cell for each client driver, should work.
Do you see any problems with that ? Either case, that doesn't seem
to be a watchdog driver problem, or am I missing something ?


Thanks,
Moritz

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/mdf/linux.git/log/?h=wip/mfd-ds1374-rfc

Moritz Fischer (2):
  rtc: ds1374: wdt: Fix issue with timeout scaling from secs to wdt
ticks
  rtc: ds1374: wdt: Fix stop/start ioctl always returning -EINVAL



I don't really see the point of doing that if you plan to move the watchdog
part of the driver into the watchdog directory. We for sure won't accept a
watchdog driver that does not use the watchdog infrastructure.

Regarding
+   /* WHY? */
+   ds1374->wdd.timeout = t;

Assuming you mean why the driver has to set the timeout value - not every
watchdog hardware supports timeouts in multiples of 1 second. The driver
is expected to set the value to the real timeout, not to the timeout asked
for by the infrastructure.

Guenter

Re: [PATCH 0/2] DS1374 Watchdog fixes

[Guenter Roeck]

On 04/24/2017 03:05 PM, Moritz Fischer wrote:

Hi all,

this series fixes two issues that I ran into when trying to use the
watchdog part of the DS1374 on of our products.

This series is basically a precursor to some further cleanup work,
that turns the DS1374 into a MFD device with an RTC and WDT in
separate drivers [1] each integrated in either the watchdog and
the rtc frameworks.

I'm very unhappy with the CONFIG_DRV_RTC_DS1374_WDT way of enabling
the watchdog behavior and currently I'm investigating how to make
that work via DT.

Watchdog maintainers, do you have an idea on how to do that in a
non breaking fashion?



Depends on what you mean with "non breaking". Just using the normal mfd
mechanisms, ie define an mfd cell for each client driver, should work.
Do you see any problems with that ? Either case, that doesn't seem
to be a watchdog driver problem, or am I missing something ?


Thanks,
Moritz

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/mdf/linux.git/log/?h=wip/mfd-ds1374-rfc

Moritz Fischer (2):
  rtc: ds1374: wdt: Fix issue with timeout scaling from secs to wdt
ticks
  rtc: ds1374: wdt: Fix stop/start ioctl always returning -EINVAL



I don't really see the point of doing that if you plan to move the watchdog
part of the driver into the watchdog directory. We for sure won't accept a
watchdog driver that does not use the watchdog infrastructure.

Regarding
+   /* WHY? */
+   ds1374->wdd.timeout = t;

Assuming you mean why the driver has to set the timeout value - not every
watchdog hardware supports timeouts in multiples of 1 second. The driver
is expected to set the value to the real timeout, not to the timeout asked
for by the infrastructure.

Guenter

Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

[Maarten Maathuis]

On Tue, Apr 25, 2017 at 4:43 AM, AceLan Kao  wrote:
> According the spec. I have, the values are correct.
> Please merge it, thanks.
>

Is there a reason the whole spec isn't implemented?
Is it under NDA?

> 2017-04-25 5:41 GMT+08:00 Maarten Maathuis :
>> On Mon, Apr 24, 2017 at 11:37 PM, Andy Shevchenko
>>  wrote:
>>> On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis  
>>> wrote:
 Tested on HP Elite X2 1012 G1.
 Matches event report of Lenovo Helix 2
 (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).

>>>
>>> Much better!
>>>
 V2: Fix indent and add sign-off
>>>
>>> Usually this line goes after --- (body delimiter).
>>> No need to resend this time. I would wait a bit for actual
>>> author/driver maintainer to comment. Otherwise patch looks good enough
>>> to me.
>>
>> The intent is not have this in the commit message?
>> I'll keep an eye out if i can place it below "---" next time.
>> Although i suspect the message would end in the actual code diff,
>> which seems odd.
>>
>>>

 Signed-off-by: Maarten Maathuis 
 ---
  drivers/platform/x86/intel-vbtn.c | 4 
  1 file changed, 4 insertions(+)

 diff --git a/drivers/platform/x86/intel-vbtn.c 
 b/drivers/platform/x86/intel-vbtn.c
 index 554e82ebe83c..1616cb9c4ae5 100644
 --- a/drivers/platform/x86/intel-vbtn.c
 +++ b/drivers/platform/x86/intel-vbtn.c
 @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
  static const struct key_entry intel_vbtn_keymap[] = {
 { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
 { KE_KEY, 0xC1, { KEY_POWER } },/* power key release */
 +   { KE_KEY, 0xC4, { KEY_VOLUMEUP} },  /* volume-up key press */
 +   { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } },  /* volume-up key release */
 +   { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } },   /* volume-down key press */
 +   { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } },/* volume-down key 
 release */
 { KE_END },
  };

 --
 2.12.2

>>>
>>>
>>>
>>> --
>>> With Best Regards,
>>> Andy Shevchenko
>>
>>
>>
>> --
>> Far away from the primal instinct, the song seems to fade away, the
>> river get wider between your thoughts and the things we do and say.



-- 
Far away from the primal instinct, the song seems to fade away, the
river get wider between your thoughts and the things we do and say.

Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

[Maarten Maathuis]

On Tue, Apr 25, 2017 at 4:43 AM, AceLan Kao  wrote:
> According the spec. I have, the values are correct.
> Please merge it, thanks.
>

Is there a reason the whole spec isn't implemented?
Is it under NDA?

> 2017-04-25 5:41 GMT+08:00 Maarten Maathuis :
>> On Mon, Apr 24, 2017 at 11:37 PM, Andy Shevchenko
>>  wrote:
>>> On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis  
>>> wrote:
 Tested on HP Elite X2 1012 G1.
 Matches event report of Lenovo Helix 2
 (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).

>>>
>>> Much better!
>>>
 V2: Fix indent and add sign-off
>>>
>>> Usually this line goes after --- (body delimiter).
>>> No need to resend this time. I would wait a bit for actual
>>> author/driver maintainer to comment. Otherwise patch looks good enough
>>> to me.
>>
>> The intent is not have this in the commit message?
>> I'll keep an eye out if i can place it below "---" next time.
>> Although i suspect the message would end in the actual code diff,
>> which seems odd.
>>
>>>

 Signed-off-by: Maarten Maathuis 
 ---
  drivers/platform/x86/intel-vbtn.c | 4 
  1 file changed, 4 insertions(+)

 diff --git a/drivers/platform/x86/intel-vbtn.c 
 b/drivers/platform/x86/intel-vbtn.c
 index 554e82ebe83c..1616cb9c4ae5 100644
 --- a/drivers/platform/x86/intel-vbtn.c
 +++ b/drivers/platform/x86/intel-vbtn.c
 @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
  static const struct key_entry intel_vbtn_keymap[] = {
 { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
 { KE_KEY, 0xC1, { KEY_POWER } },/* power key release */
 +   { KE_KEY, 0xC4, { KEY_VOLUMEUP} },  /* volume-up key press */
 +   { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } },  /* volume-up key release */
 +   { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } },   /* volume-down key press */
 +   { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } },/* volume-down key 
 release */
 { KE_END },
  };

 --
 2.12.2

>>>
>>>
>>>
>>> --
>>> With Best Regards,
>>> Andy Shevchenko
>>
>>
>>
>> --
>> Far away from the primal instinct, the song seems to fade away, the
>> river get wider between your thoughts and the things we do and say.



-- 
Far away from the primal instinct, the song seems to fade away, the
river get wider between your thoughts and the things we do and say.

Re: [PATCH V3 11/17] thermal: cpu_cooling: get rid of 'allowed_cpus'

[Viresh Kumar]

On 24-04-17, 17:53, Lukasz Luba wrote:
> The policy pointer forwarded from cpufreq_update_policy()
> is a local variable 'new_policy' so cannot be compared with pinned
> policy pointer in the cooling device.
> You should do the cpumask test like before:
>   if (!cpumask_test_cpu(policy->cpu,
> cpufreq_cdev->policy->related_cpus))

Right. I have fixed it a bit differently now.

> But there is something still in the patch set...
> I will try to check it tomorrow.

I reviewed all the patches very carefully again, trying to find out the culprit
(I don't have the right hardware to test it like you have).

Found out that max_level isn't used properly at few places, fixed and pushed my
branch now. See if it works fine now.

HEAD: 6a883ddf73cd

-- 
viresh

Re: [PATCH V3 11/17] thermal: cpu_cooling: get rid of 'allowed_cpus'

[Viresh Kumar]

On 24-04-17, 17:53, Lukasz Luba wrote:
> The policy pointer forwarded from cpufreq_update_policy()
> is a local variable 'new_policy' so cannot be compared with pinned
> policy pointer in the cooling device.
> You should do the cpumask test like before:
>   if (!cpumask_test_cpu(policy->cpu,
> cpufreq_cdev->policy->related_cpus))

Right. I have fixed it a bit differently now.

> But there is something still in the patch set...
> I will try to check it tomorrow.

I reviewed all the patches very carefully again, trying to find out the culprit
(I don't have the right hardware to test it like you have).

Found out that max_level isn't used properly at few places, fixed and pushed my
branch now. See if it works fine now.

HEAD: 6a883ddf73cd

-- 
viresh

Re: [PATCH -next] phy: qcom-qmp: fix return value check in qcom_qmp_phy_create()

[Vivek Gautam]

On 04/25/2017 08:44 AM, Wei Yongjun wrote:

From: Wei Yongjun 

In case of error, the function of_iomap() returns NULL pointer
not ERR_PTR(). The IS_ERR() test in the return value check should
be replaced with NULL test.

Fixes: e78f3d15e115 ("phy: qcom-qmp: new qmp phy driver for qcom-chipsets")
Signed-off-by: Wei Yongjun 


Reviewed-by: Vivek Gautam 


---
  drivers/phy/phy-qcom-qmp.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/phy/phy-qcom-qmp.c b/drivers/phy/phy-qcom-qmp.c
index 727e23b..a25c29d 100644
--- a/drivers/phy/phy-qcom-qmp.c
+++ b/drivers/phy/phy-qcom-qmp.c
@@ -983,16 +983,16 @@ int qcom_qmp_phy_create(struct device *dev, struct 
device_node *np, int id)
 * Resources are indexed as: tx -> 0; rx -> 1; pcs -> 2.
 */
qphy->tx = of_iomap(np, 0);
-   if (IS_ERR(qphy->tx))
-   return PTR_ERR(qphy->tx);
+   if (!qphy->tx)
+   return -ENOMEM;
  
  	qphy->rx = of_iomap(np, 1);

-   if (IS_ERR(qphy->rx))
-   return PTR_ERR(qphy->rx);
+   if (!qphy->rx)
+   return -ENOMEM;
  
  	qphy->pcs = of_iomap(np, 2);

-   if (IS_ERR(qphy->pcs))
-   return PTR_ERR(qphy->pcs);
+   if (!qphy->pcs)
+   return -ENOMEM;
  
  	/*

 * Get PHY's Pipe clock, if any. USB3 and PCIe are PIPE3



--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH -next] phy: qcom-qmp: fix return value check in qcom_qmp_phy_create()

[Vivek Gautam]

On 04/25/2017 08:44 AM, Wei Yongjun wrote:

From: Wei Yongjun 

In case of error, the function of_iomap() returns NULL pointer
not ERR_PTR(). The IS_ERR() test in the return value check should
be replaced with NULL test.

Fixes: e78f3d15e115 ("phy: qcom-qmp: new qmp phy driver for qcom-chipsets")
Signed-off-by: Wei Yongjun 


Reviewed-by: Vivek Gautam 


---
  drivers/phy/phy-qcom-qmp.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/phy/phy-qcom-qmp.c b/drivers/phy/phy-qcom-qmp.c
index 727e23b..a25c29d 100644
--- a/drivers/phy/phy-qcom-qmp.c
+++ b/drivers/phy/phy-qcom-qmp.c
@@ -983,16 +983,16 @@ int qcom_qmp_phy_create(struct device *dev, struct 
device_node *np, int id)
 * Resources are indexed as: tx -> 0; rx -> 1; pcs -> 2.
 */
qphy->tx = of_iomap(np, 0);
-   if (IS_ERR(qphy->tx))
-   return PTR_ERR(qphy->tx);
+   if (!qphy->tx)
+   return -ENOMEM;
  
  	qphy->rx = of_iomap(np, 1);

-   if (IS_ERR(qphy->rx))
-   return PTR_ERR(qphy->rx);
+   if (!qphy->rx)
+   return -ENOMEM;
  
  	qphy->pcs = of_iomap(np, 2);

-   if (IS_ERR(qphy->pcs))
-   return PTR_ERR(qphy->pcs);
+   if (!qphy->pcs)
+   return -ENOMEM;
  
  	/*

 * Get PHY's Pipe clock, if any. USB3 and PCIe are PIPE3



--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH V3 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Vivek Gautam]

On 04/24/2017 06:15 PM, Jon Hunter wrote:

On 18/04/17 12:21, Vivek Gautam wrote:

Make use of reset_control_array_*() set of APIs to manage
an array of reset controllers available with the device.

Before we apply this patch, I need to check to see if the order of the
resets managed by the PMC driver matter. Today the order of the resets
is determined by the order they appear in the DT node and although the
new APIs work in the same way they do not guarantee this. So let me
check to see if we can any concerns about ordering here. Otherwise would
be nice to use these APIs.


Right, that will be perfect.

Best regards
Vivek



Cheers
Jon



--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH V3 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Vivek Gautam]

On 04/24/2017 06:15 PM, Jon Hunter wrote:

On 18/04/17 12:21, Vivek Gautam wrote:

Make use of reset_control_array_*() set of APIs to manage
an array of reset controllers available with the device.

Before we apply this patch, I need to check to see if the order of the
resets managed by the PMC driver matter. Today the order of the resets
is determined by the order they appear in the DT node and although the
new APIs work in the same way they do not guarantee this. So let me
check to see if we can any concerns about ordering here. Otherwise would
be nice to use these APIs.


Right, that will be perfect.

Best regards
Vivek



Cheers
Jon



--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v5 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Matt Brown]

This introduces the tiocsti_restrict sysctl, whose default is controlled via
CONFIG_SECURITY_TIOCSTI_RESTRICT. When activated, this control restricts
all TIOCSTI ioctl calls from non CAP_SYS_ADMIN users.

This patch depends on patch 1/2

This patch was inspired from GRKERNSEC_HARDEN_TTY.

This patch would have prevented
https://bugzilla.redhat.com/show_bug.cgi?id=1411256 under the following
conditions:
* non-privileged container
* container run inside new user namespace

Possible effects on userland:

There could be a few user programs that would be effected by this
change.
See: 
notable programs are: agetty, csh, xemacs and tcsh

However, I still believe that this change is worth it given that the
Kconfig defaults to n. This will be a feature that is turned on for the
same reason that people activate it when using grsecurity. Users of this
opt-in feature will realize that they are choosing security over some OS
features like unprivileged TIOCSTI ioctls, as should be clear in the
Kconfig help message.

Threat Model/Patch Rational:

>From grsecurity's config for GRKERNSEC_HARDEN_TTY.

 | There are very few legitimate uses for this functionality and it
 | has made vulnerabilities in several 'su'-like programs possible in
 | the past.  Even without these vulnerabilities, it provides an
 | attacker with an easy mechanism to move laterally among other
 | processes within the same user's compromised session.

So if one process within a tty session becomes compromised it can follow
that additional processes, that are thought to be in different security
boundaries, can be compromised as a result. When using a program like su
or sudo, these additional processes could be in a tty session where TTY file
descriptors are indeed shared over privilege boundaries.

This is also an excellent writeup about the issue:


When user namespaces are in use, the check for the capability
CAP_SYS_ADMIN is done against the user namespace that originally opened
the tty.

Signed-off-by: Matt Brown 
---
 Documentation/sysctl/kernel.txt | 21 +
 drivers/tty/tty_io.c|  6 ++
 include/linux/tty.h |  2 ++
 kernel/sysctl.c | 12 
 security/Kconfig| 13 +
 5 files changed, 54 insertions(+)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index bac23c1..f7985cf 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -89,6 +89,7 @@ show up in /proc/sys/kernel:
 - sysctl_writes_strict
 - tainted
 - threads-max
+- tiocsti_restrict
 - unknown_nmi_panic
 - watchdog
 - watchdog_thresh
@@ -987,6 +988,26 @@ available RAM pages threads-max is reduced accordingly.
 
 ==
 
+tiocsti_restrict:
+
+This toggle indicates whether unprivileged users are prevented
+from using the TIOCSTI ioctl to inject commands into other processes
+which share a tty session.
+
+When tiocsti_restrict is set to (0) there are no restrictions(accept
+the default restriction of only being able to injection commands into
+one's own tty). When tiocsti_restrict is set to (1), users must
+have CAP_SYS_ADMIN to use the TIOCSTI ioctl.
+
+When user namespaces are in use, the check for the capability
+CAP_SYS_ADMIN is done against the user namespace that originally
+opened the tty.
+
+The kernel config option CONFIG_SECURITY_TIOCSTI_RESTRICT sets the
+default value of tiocsti_restrict.
+
+==
+
 unknown_nmi_panic:
 
 The value in this file affects behavior of handling NMI. When the
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index c276814..fe68d14 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2297,11 +2297,17 @@ static int tty_fasync(int fd, struct file *filp, int on)
  * FIXME: may race normal receive processing
  */
 
+int tiocsti_restrict = IS_ENABLED(CONFIG_SECURITY_TIOCSTI_RESTRICT);
+
 static int tiocsti(struct tty_struct *tty, char __user *p)
 {
char ch, mbz = 0;
struct tty_ldisc *ld;
 
+   if (tiocsti_restrict && !ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)) {
+   pr_warn_ratelimited("TIOCSTI ioctl call blocked for 
non-privileged process\n");
+   return -EPERM;
+   }
if ((current->signal->tty != tty) && !capable(CAP_SYS_ADMIN))
return -EPERM;
if (get_user(ch, p))
diff --git a/include/linux/tty.h b/include/linux/tty.h
index d902d42..2fd7f49 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -344,6 +344,8 @@ struct tty_file_private {
struct list_head list;
 };
 
+extern int tiocsti_restrict;
+
 /* tty magic number */
 #define TTY_MAGIC  0x5401
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index acf0a5a..68d1363 

[PATCH v5 1/2] security: tty: Add owner user namespace to tty_struct

[Matt Brown]

This patch adds struct user_namespace *owner_user_ns to the tty_struct.
Then it is set to current_user_ns() in the alloc_tty_struct function.

This is done to facilitate capability checks against the original user
namespace that allocated the tty.

E.g. ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)

This combined with the use of user namespace's will allow hardening
protections to be built to mitigate container escapes that utilize TTY
ioctls such as TIOCSTI.

See: https://bugzilla.redhat.com/show_bug.cgi?id=1411256

Signed-off-by: Matt Brown 
---
 drivers/tty/tty_io.c | 2 ++
 include/linux/tty.h  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e6d1a65..c276814 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -171,6 +171,7 @@ static void free_tty_struct(struct tty_struct *tty)
put_device(tty->dev);
kfree(tty->write_buf);
tty->magic = 0xDEADDEAD;
+   put_user_ns(tty->owner_user_ns);
kfree(tty);
 }
 
@@ -3191,6 +3192,7 @@ struct tty_struct *alloc_tty_struct(struct tty_driver 
*driver, int idx)
tty->index = idx;
tty_line_name(driver, idx, tty->name);
tty->dev = tty_get_device(tty);
+   tty->owner_user_ns = get_user_ns(current_user_ns());
 
return tty;
 }
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1017e904..d902d42 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 
 /*
@@ -333,6 +334,7 @@ struct tty_struct {
/* If the tty has a pending do_SAK, queue it here - akpm */
struct work_struct SAK_work;
struct tty_port *port;
+   struct user_namespace *owner_user_ns;
 };
 
 /* Each of a tty's open files has private_data pointing to tty_file_private */
-- 
2.10.2

[PATCH v5 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Matt Brown]

This introduces the tiocsti_restrict sysctl, whose default is controlled via
CONFIG_SECURITY_TIOCSTI_RESTRICT. When activated, this control restricts
all TIOCSTI ioctl calls from non CAP_SYS_ADMIN users.

This patch depends on patch 1/2

This patch was inspired from GRKERNSEC_HARDEN_TTY.

This patch would have prevented
https://bugzilla.redhat.com/show_bug.cgi?id=1411256 under the following
conditions:
* non-privileged container
* container run inside new user namespace

Possible effects on userland:

There could be a few user programs that would be effected by this
change.
See: 
notable programs are: agetty, csh, xemacs and tcsh

However, I still believe that this change is worth it given that the
Kconfig defaults to n. This will be a feature that is turned on for the
same reason that people activate it when using grsecurity. Users of this
opt-in feature will realize that they are choosing security over some OS
features like unprivileged TIOCSTI ioctls, as should be clear in the
Kconfig help message.

Threat Model/Patch Rational:

>From grsecurity's config for GRKERNSEC_HARDEN_TTY.

 | There are very few legitimate uses for this functionality and it
 | has made vulnerabilities in several 'su'-like programs possible in
 | the past.  Even without these vulnerabilities, it provides an
 | attacker with an easy mechanism to move laterally among other
 | processes within the same user's compromised session.

So if one process within a tty session becomes compromised it can follow
that additional processes, that are thought to be in different security
boundaries, can be compromised as a result. When using a program like su
or sudo, these additional processes could be in a tty session where TTY file
descriptors are indeed shared over privilege boundaries.

This is also an excellent writeup about the issue:


When user namespaces are in use, the check for the capability
CAP_SYS_ADMIN is done against the user namespace that originally opened
the tty.

Signed-off-by: Matt Brown 
---
 Documentation/sysctl/kernel.txt | 21 +
 drivers/tty/tty_io.c|  6 ++
 include/linux/tty.h |  2 ++
 kernel/sysctl.c | 12 
 security/Kconfig| 13 +
 5 files changed, 54 insertions(+)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index bac23c1..f7985cf 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -89,6 +89,7 @@ show up in /proc/sys/kernel:
 - sysctl_writes_strict
 - tainted
 - threads-max
+- tiocsti_restrict
 - unknown_nmi_panic
 - watchdog
 - watchdog_thresh
@@ -987,6 +988,26 @@ available RAM pages threads-max is reduced accordingly.
 
 ==
 
+tiocsti_restrict:
+
+This toggle indicates whether unprivileged users are prevented
+from using the TIOCSTI ioctl to inject commands into other processes
+which share a tty session.
+
+When tiocsti_restrict is set to (0) there are no restrictions(accept
+the default restriction of only being able to injection commands into
+one's own tty). When tiocsti_restrict is set to (1), users must
+have CAP_SYS_ADMIN to use the TIOCSTI ioctl.
+
+When user namespaces are in use, the check for the capability
+CAP_SYS_ADMIN is done against the user namespace that originally
+opened the tty.
+
+The kernel config option CONFIG_SECURITY_TIOCSTI_RESTRICT sets the
+default value of tiocsti_restrict.
+
+==
+
 unknown_nmi_panic:
 
 The value in this file affects behavior of handling NMI. When the
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index c276814..fe68d14 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2297,11 +2297,17 @@ static int tty_fasync(int fd, struct file *filp, int on)
  * FIXME: may race normal receive processing
  */
 
+int tiocsti_restrict = IS_ENABLED(CONFIG_SECURITY_TIOCSTI_RESTRICT);
+
 static int tiocsti(struct tty_struct *tty, char __user *p)
 {
char ch, mbz = 0;
struct tty_ldisc *ld;
 
+   if (tiocsti_restrict && !ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)) {
+   pr_warn_ratelimited("TIOCSTI ioctl call blocked for 
non-privileged process\n");
+   return -EPERM;
+   }
if ((current->signal->tty != tty) && !capable(CAP_SYS_ADMIN))
return -EPERM;
if (get_user(ch, p))
diff --git a/include/linux/tty.h b/include/linux/tty.h
index d902d42..2fd7f49 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -344,6 +344,8 @@ struct tty_file_private {
struct list_head list;
 };
 
+extern int tiocsti_restrict;
+
 /* tty magic number */
 #define TTY_MAGIC  0x5401
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index acf0a5a..68d1363 100644
--- 

[PATCH v5 1/2] security: tty: Add owner user namespace to tty_struct

[Matt Brown]

This patch adds struct user_namespace *owner_user_ns to the tty_struct.
Then it is set to current_user_ns() in the alloc_tty_struct function.

This is done to facilitate capability checks against the original user
namespace that allocated the tty.

E.g. ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)

This combined with the use of user namespace's will allow hardening
protections to be built to mitigate container escapes that utilize TTY
ioctls such as TIOCSTI.

See: https://bugzilla.redhat.com/show_bug.cgi?id=1411256

Signed-off-by: Matt Brown 
---
 drivers/tty/tty_io.c | 2 ++
 include/linux/tty.h  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e6d1a65..c276814 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -171,6 +171,7 @@ static void free_tty_struct(struct tty_struct *tty)
put_device(tty->dev);
kfree(tty->write_buf);
tty->magic = 0xDEADDEAD;
+   put_user_ns(tty->owner_user_ns);
kfree(tty);
 }
 
@@ -3191,6 +3192,7 @@ struct tty_struct *alloc_tty_struct(struct tty_driver 
*driver, int idx)
tty->index = idx;
tty_line_name(driver, idx, tty->name);
tty->dev = tty_get_device(tty);
+   tty->owner_user_ns = get_user_ns(current_user_ns());
 
return tty;
 }
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1017e904..d902d42 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 
 /*
@@ -333,6 +334,7 @@ struct tty_struct {
/* If the tty has a pending do_SAK, queue it here - akpm */
struct work_struct SAK_work;
struct tty_port *port;
+   struct user_namespace *owner_user_ns;
 };
 
 /* Each of a tty's open files has private_data pointing to tty_file_private */
-- 
2.10.2

[PATCH v5 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Matt Brown]

This patchset introduces the tiocsti_restrict sysctl, whose default is
controlled via CONFIG_SECURITY_TIOCSTI_RESTRICT. When activated, this
control restricts all TIOCSTI ioctl calls from non CAP_SYS_ADMIN users.

This patch was inspired from GRKERNSEC_HARDEN_TTY.

This patch would have prevented
https://bugzilla.redhat.com/show_bug.cgi?id=1411256 under the following
conditions:
* non-privileged container
* container run inside new user namespace

Possible effects on userland:

There could be a few user programs that would be effected by this
change.
See: 
notable programs are: agetty, csh, xemacs and tcsh

However, I still believe that this change is worth it given that the
Kconfig defaults to n. This will be a feature that is turned on for the
same reason that people activate it when using grsecurity. Users of this
opt-in feature will realize that they are choosing security over some OS
features like unprivileged TIOCSTI ioctls, as should be clear in the
Kconfig help message.

Threat Model/Patch Rational:

>From grsecurity's config for GRKERNSEC_HARDEN_TTY.

 | There are very few legitimate uses for this functionality and it
 | has made vulnerabilities in several 'su'-like programs possible in
 | the past.  Even without these vulnerabilities, it provides an
 | attacker with an easy mechanism to move laterally among other
 | processes within the same user's compromised session.

So if one process within a tty session becomes compromised it can follow
that additional processes, that are thought to be in different security
boundaries, can be compromised as a result. When using a program like su
or sudo, these additional processes could be in a tty session where TTY file
descriptors are indeed shared over privilege boundaries.

This is also an excellent writeup about the issue:


When user namespaces are in use, the check for the capability
CAP_SYS_ADMIN is done against the user namespace that originally opened
the tty.

# Changes since v4:
* fixed typo

# Changes since v3:
* use get_user_ns and put_user_ns to take and drop references to the owner
  user namespace because CONFIG_USER_NS is an option

# Changes since v2:
* take/drop reference to user namespace on tty struct alloc/free to prevent
  use-after-free.

# Changes since v1:
* added owner_user_ns to tty_struct to enable capability checks against
  the namespace that created the tty.
* rewording in different places to make patchset purpose clear
* Added Documentation

[PATCH v5 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Matt Brown]

This patchset introduces the tiocsti_restrict sysctl, whose default is
controlled via CONFIG_SECURITY_TIOCSTI_RESTRICT. When activated, this
control restricts all TIOCSTI ioctl calls from non CAP_SYS_ADMIN users.

This patch was inspired from GRKERNSEC_HARDEN_TTY.

This patch would have prevented
https://bugzilla.redhat.com/show_bug.cgi?id=1411256 under the following
conditions:
* non-privileged container
* container run inside new user namespace

Possible effects on userland:

There could be a few user programs that would be effected by this
change.
See: 
notable programs are: agetty, csh, xemacs and tcsh

However, I still believe that this change is worth it given that the
Kconfig defaults to n. This will be a feature that is turned on for the
same reason that people activate it when using grsecurity. Users of this
opt-in feature will realize that they are choosing security over some OS
features like unprivileged TIOCSTI ioctls, as should be clear in the
Kconfig help message.

Threat Model/Patch Rational:

>From grsecurity's config for GRKERNSEC_HARDEN_TTY.

 | There are very few legitimate uses for this functionality and it
 | has made vulnerabilities in several 'su'-like programs possible in
 | the past.  Even without these vulnerabilities, it provides an
 | attacker with an easy mechanism to move laterally among other
 | processes within the same user's compromised session.

So if one process within a tty session becomes compromised it can follow
that additional processes, that are thought to be in different security
boundaries, can be compromised as a result. When using a program like su
or sudo, these additional processes could be in a tty session where TTY file
descriptors are indeed shared over privilege boundaries.

This is also an excellent writeup about the issue:


When user namespaces are in use, the check for the capability
CAP_SYS_ADMIN is done against the user namespace that originally opened
the tty.

# Changes since v4:
* fixed typo

# Changes since v3:
* use get_user_ns and put_user_ns to take and drop references to the owner
  user namespace because CONFIG_USER_NS is an option

# Changes since v2:
* take/drop reference to user namespace on tty struct alloc/free to prevent
  use-after-free.

# Changes since v1:
* added owner_user_ns to tty_struct to enable capability checks against
  the namespace that created the tty.
* rewording in different places to make patchset purpose clear
* Added Documentation

Re: [PATCH v2] fpga: allow to compile-test Altera FPGA bridge drivers

[kbuild test robot]

Hi Tobias,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.11-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Tobias-Klauser/fpga-allow-to-compile-test-Altera-FPGA-bridge-drivers/20170412-180901
config: um-allyesconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=um 

All errors (new ones prefixed by >>):

   arch/um/drivers/built-in.o: In function `vde_open_real':
   (.text+0xc9f1): warning: Using 'getgrnam' in statically linked applications 
requires at runtime the shared libraries from the glibc version used for linking
   arch/um/drivers/built-in.o: In function `vde_open_real':
   (.text+0xc83c): warning: Using 'getpwuid' in statically linked applications 
requires at runtime the shared libraries from the glibc version used for linking
   arch/um/drivers/built-in.o: In function `vde_open_real':
   (.text+0xcb55): warning: Using 'getaddrinfo' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
   arch/um/drivers/built-in.o: In function `pcap_nametoaddr':
   (.text+0x1d5e5): warning: Using 'gethostbyname' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
   arch/um/drivers/built-in.o: In function `pcap_nametonetaddr':
   (.text+0x1d685): warning: Using 'getnetbyname' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
   arch/um/drivers/built-in.o: In function `pcap_nametoproto':
   (.text+0x1d8a5): warning: Using 'getprotobyname' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
   arch/um/drivers/built-in.o: In function `pcap_nametoport':
   (.text+0x1d6d7): warning: Using 'getservbyname' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
   drivers/built-in.o: In function `img_ascii_lcd_probe':
   drivers/auxdisplay/img-ascii-lcd.c:385: undefined reference to 
`devm_ioremap_resource'
   drivers/built-in.o: In function `altera_freeze_br_probe':
>> drivers/fpga/altera-freeze-bridge.c:235: undefined reference to 
>> `devm_ioremap_resource'
   collect2: error: ld returned 1 exit status

vim +235 drivers/fpga/altera-freeze-bridge.c

ca24a648 Alan Tull 2016-11-01  219  struct device *dev = >dev;
ca24a648 Alan Tull 2016-11-01  220  struct device_node *np = 
pdev->dev.of_node;
ca24a648 Alan Tull 2016-11-01  221  struct altera_freeze_br_data *priv;
ca24a648 Alan Tull 2016-11-01  222  struct resource *res;
ca24a648 Alan Tull 2016-11-01  223  u32 status, revision;
ca24a648 Alan Tull 2016-11-01  224  
ca24a648 Alan Tull 2016-11-01  225  if (!np)
ca24a648 Alan Tull 2016-11-01  226  return -ENODEV;
ca24a648 Alan Tull 2016-11-01  227  
ca24a648 Alan Tull 2016-11-01  228  priv = devm_kzalloc(dev, sizeof(*priv), 
GFP_KERNEL);
ca24a648 Alan Tull 2016-11-01  229  if (!priv)
ca24a648 Alan Tull 2016-11-01  230  return -ENOMEM;
ca24a648 Alan Tull 2016-11-01  231  
ca24a648 Alan Tull 2016-11-01  232  priv->dev = dev;
ca24a648 Alan Tull 2016-11-01  233  
ca24a648 Alan Tull 2016-11-01  234  res = platform_get_resource(pdev, 
IORESOURCE_MEM, 0);
ca24a648 Alan Tull 2016-11-01 @235  priv->base_addr = 
devm_ioremap_resource(dev, res);
ca24a648 Alan Tull 2016-11-01  236  if (IS_ERR(priv->base_addr))
ca24a648 Alan Tull 2016-11-01  237  return PTR_ERR(priv->base_addr);
ca24a648 Alan Tull 2016-11-01  238  
ca24a648 Alan Tull 2016-11-01  239  status = readl(priv->base_addr + 
FREEZE_CSR_STATUS_OFFSET);
ca24a648 Alan Tull 2016-11-01  240  if (status & 
FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE)
ca24a648 Alan Tull 2016-11-01  241  priv->enable = 1;
ca24a648 Alan Tull 2016-11-01  242  
ca24a648 Alan Tull 2016-11-01  243  revision = readl(priv->base_addr + 
FREEZE_CSR_REG_VERSION);

:: The code at line 235 was first introduced by commit
:: ca24a648f535a02b4163ca4f4d2e51869f155a3a fpga: add altera freeze bridge 
support

:: TO: Alan Tull 
:: CC: Greg Kroah-Hartman 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] ARM: dts: Add devicetree for the Raspberry Pi 3, for arm32 (v5)

[Florian Fainelli]

On 04/24/2017 01:00 PM, Eric Anholt wrote:
> Raspbian and Fedora have decided to support the Pi3 in 32-bit mode for
> now, so it's useful to be able to test that mode on an upstream
> kernel.  It's also been useful for me to use the same board for 32-bit
> and 64-bit development.
> 
> Signed-off-by: Eric Anholt 
> ---
>  arch/arm/boot/dts/Makefile| 1 +
>  arch/arm/boot/dts/bcm2837-rpi-3.b.dts | 1 +
>  2 files changed, 2 insertions(+)
>  create mode 100644 arch/arm/boot/dts/bcm2837-rpi-3.b.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 011808490fed..eded842d9978 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -72,6 +72,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
>   bcm2835-rpi-b-plus.dtb \
>   bcm2835-rpi-a-plus.dtb \
>   bcm2836-rpi-2-b.dtb \
> + bcm2837-rpi-3-b.dtb \
>   bcm2835-rpi-zero.dtb
>  dtb-$(CONFIG_ARCH_BCM_5301X) += \
>   bcm4708-asus-rt-ac56u.dtb \
> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3.b.dts 
> b/arch/arm/boot/dts/bcm2837-rpi-3.b.dts
> new file mode 100644
> index ..8c8aa4d1e9b3
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm2837-rpi-3.b.dts
> @@ -0,0 +1 @@
> +#include "arm64/broadcom/bcm2837-rpi-3.b.dts"

Sounds like this should be
../arm64/boot/dts/broadcom/bcm2837-rpi-3.b.dts are reported by the
kbuild test robot?
-- 
Florian

Re: [PATCH v2] fpga: allow to compile-test Altera FPGA bridge drivers

[kbuild test robot]

Hi Tobias,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.11-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Tobias-Klauser/fpga-allow-to-compile-test-Altera-FPGA-bridge-drivers/20170412-180901
config: um-allyesconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=um 

All errors (new ones prefixed by >>):

   arch/um/drivers/built-in.o: In function `vde_open_real':
   (.text+0xc9f1): warning: Using 'getgrnam' in statically linked applications 
requires at runtime the shared libraries from the glibc version used for linking
   arch/um/drivers/built-in.o: In function `vde_open_real':
   (.text+0xc83c): warning: Using 'getpwuid' in statically linked applications 
requires at runtime the shared libraries from the glibc version used for linking
   arch/um/drivers/built-in.o: In function `vde_open_real':
   (.text+0xcb55): warning: Using 'getaddrinfo' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
   arch/um/drivers/built-in.o: In function `pcap_nametoaddr':
   (.text+0x1d5e5): warning: Using 'gethostbyname' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
   arch/um/drivers/built-in.o: In function `pcap_nametonetaddr':
   (.text+0x1d685): warning: Using 'getnetbyname' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
   arch/um/drivers/built-in.o: In function `pcap_nametoproto':
   (.text+0x1d8a5): warning: Using 'getprotobyname' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
   arch/um/drivers/built-in.o: In function `pcap_nametoport':
   (.text+0x1d6d7): warning: Using 'getservbyname' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
   drivers/built-in.o: In function `img_ascii_lcd_probe':
   drivers/auxdisplay/img-ascii-lcd.c:385: undefined reference to 
`devm_ioremap_resource'
   drivers/built-in.o: In function `altera_freeze_br_probe':
>> drivers/fpga/altera-freeze-bridge.c:235: undefined reference to 
>> `devm_ioremap_resource'
   collect2: error: ld returned 1 exit status

vim +235 drivers/fpga/altera-freeze-bridge.c

ca24a648 Alan Tull 2016-11-01  219  struct device *dev = >dev;
ca24a648 Alan Tull 2016-11-01  220  struct device_node *np = 
pdev->dev.of_node;
ca24a648 Alan Tull 2016-11-01  221  struct altera_freeze_br_data *priv;
ca24a648 Alan Tull 2016-11-01  222  struct resource *res;
ca24a648 Alan Tull 2016-11-01  223  u32 status, revision;
ca24a648 Alan Tull 2016-11-01  224  
ca24a648 Alan Tull 2016-11-01  225  if (!np)
ca24a648 Alan Tull 2016-11-01  226  return -ENODEV;
ca24a648 Alan Tull 2016-11-01  227  
ca24a648 Alan Tull 2016-11-01  228  priv = devm_kzalloc(dev, sizeof(*priv), 
GFP_KERNEL);
ca24a648 Alan Tull 2016-11-01  229  if (!priv)
ca24a648 Alan Tull 2016-11-01  230  return -ENOMEM;
ca24a648 Alan Tull 2016-11-01  231  
ca24a648 Alan Tull 2016-11-01  232  priv->dev = dev;
ca24a648 Alan Tull 2016-11-01  233  
ca24a648 Alan Tull 2016-11-01  234  res = platform_get_resource(pdev, 
IORESOURCE_MEM, 0);
ca24a648 Alan Tull 2016-11-01 @235  priv->base_addr = 
devm_ioremap_resource(dev, res);
ca24a648 Alan Tull 2016-11-01  236  if (IS_ERR(priv->base_addr))
ca24a648 Alan Tull 2016-11-01  237  return PTR_ERR(priv->base_addr);
ca24a648 Alan Tull 2016-11-01  238  
ca24a648 Alan Tull 2016-11-01  239  status = readl(priv->base_addr + 
FREEZE_CSR_STATUS_OFFSET);
ca24a648 Alan Tull 2016-11-01  240  if (status & 
FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE)
ca24a648 Alan Tull 2016-11-01  241  priv->enable = 1;
ca24a648 Alan Tull 2016-11-01  242  
ca24a648 Alan Tull 2016-11-01  243  revision = readl(priv->base_addr + 
FREEZE_CSR_REG_VERSION);

:: The code at line 235 was first introduced by commit
:: ca24a648f535a02b4163ca4f4d2e51869f155a3a fpga: add altera freeze bridge 
support

:: TO: Alan Tull 
:: CC: Greg Kroah-Hartman 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] ARM: dts: Add devicetree for the Raspberry Pi 3, for arm32 (v5)

[Florian Fainelli]

On 04/24/2017 01:00 PM, Eric Anholt wrote:
> Raspbian and Fedora have decided to support the Pi3 in 32-bit mode for
> now, so it's useful to be able to test that mode on an upstream
> kernel.  It's also been useful for me to use the same board for 32-bit
> and 64-bit development.
> 
> Signed-off-by: Eric Anholt 
> ---
>  arch/arm/boot/dts/Makefile| 1 +
>  arch/arm/boot/dts/bcm2837-rpi-3.b.dts | 1 +
>  2 files changed, 2 insertions(+)
>  create mode 100644 arch/arm/boot/dts/bcm2837-rpi-3.b.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 011808490fed..eded842d9978 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -72,6 +72,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
>   bcm2835-rpi-b-plus.dtb \
>   bcm2835-rpi-a-plus.dtb \
>   bcm2836-rpi-2-b.dtb \
> + bcm2837-rpi-3-b.dtb \
>   bcm2835-rpi-zero.dtb
>  dtb-$(CONFIG_ARCH_BCM_5301X) += \
>   bcm4708-asus-rt-ac56u.dtb \
> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3.b.dts 
> b/arch/arm/boot/dts/bcm2837-rpi-3.b.dts
> new file mode 100644
> index ..8c8aa4d1e9b3
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm2837-rpi-3.b.dts
> @@ -0,0 +1 @@
> +#include "arm64/broadcom/bcm2837-rpi-3.b.dts"

Sounds like this should be
../arm64/boot/dts/broadcom/bcm2837-rpi-3.b.dts are reported by the
kbuild test robot?
-- 
Florian

Re: [PATCH] virtio_blk: Fix English in description of VIRTIO_BLK_SCSI

[Jens Axboe]

On 04/24/2017 12:13 AM, Jean Delvare wrote:
> Signed-off-by: Jean Delvare 
> Fixes: 97b50a654d5d ("virtio_blk: make SCSI passthrough support configurable")
> Cc: Christoph Hellwig 
> Cc: Jens Axboe 

Added for 4.12, thanks.

-- 
Jens Axboe

Re: [PATCH] virtio_blk: Fix English in description of VIRTIO_BLK_SCSI

[Jens Axboe]

On 04/24/2017 12:13 AM, Jean Delvare wrote:
> Signed-off-by: Jean Delvare 
> Fixes: 97b50a654d5d ("virtio_blk: make SCSI passthrough support configurable")
> Cc: Christoph Hellwig 
> Cc: Jens Axboe 

Added for 4.12, thanks.

-- 
Jens Axboe

Re: [PATCH RFC] ptr_ring: add ptr_ring_unconsume

[Jason Wang]

On 2017年04月24日 20:00, Michael S. Tsirkin wrote:

On Mon, Apr 24, 2017 at 07:54:18PM +0800, Jason Wang wrote:

On 2017年04月24日 07:28, Michael S. Tsirkin wrote:

On Tue, Apr 18, 2017 at 11:07:42AM +0800, Jason Wang wrote:

On 2017年04月17日 07:19, Michael S. Tsirkin wrote:

Applications that consume a batch of entries in one go
can benefit from ability to return some of them back
into the ring.

Add an API for that - assuming there's space. If there's no space
naturally we can't do this and have to drop entries, but this implies
ring is full so we'd likely drop some anyway.

Signed-off-by: Michael S. Tsirkin
---

Jason, in my mind the biggest issue with your batching patchset is the
backet drops on disconnect.  This API will help avoid that in the common
case.

Ok, I will rebase the series on top of this. (Though I don't think we care
the packet loss).

E.g. I care - I often start sending packets to VM before it's
fully booted. Several vhost resets might follow.

Ok.


I would still prefer that we understand what's going on,

I try to reply in another thread, does it make sense?


and I would
like to know what's the smallest batch size that's still helpful,

Yes, I've replied in another thread, the result is:


no batching   1.88Mpps
RX_BATCH=11.93Mpps
RX_BATCH=42.11Mpps
RX_BATCH=16   2.14Mpps
RX_BATCH=64   2.25Mpps
RX_BATCH=256  2.18Mpps

Essentially 4 is enough, other stuf looks more like noise
to me. What about 2?

The numbers are pretty stable, so probably not noise. Retested on top of
batch zeroing:

no  1.97Mpps
1   2.09Mpps
2   2.11Mpps
4   2.16Mpps
8   2.19Mpps
16  2.21Mpps
32  2.25Mpps
64  2.30Mpps
128 2.21Mpps
256 2.21Mpps

64 performs best.

Thanks

OK but it might be e.g. a function of the ring size, host cache size or
whatever. As we don't really understand the why, if we just optimize for
your setup we risk regressions in others.  64 entries is a lot, it
increases the queue size noticeably.  Could this be part of the effect?
Could you try changing the queue size to see what happens?


I increase tx_queue_len to 1100, but only see less than 1% improvement 
on pps number (batch = 1) in my machine. If you care about the 
regression, we probably can leave the choice to user through e.g module 
parameter. But I'm afraid we have already had too much choices for them. 
Or I can test this with different CPU types.


Thanks

Re: [PATCH RFC] ptr_ring: add ptr_ring_unconsume

[Jason Wang]

On 2017年04月24日 20:00, Michael S. Tsirkin wrote:

On Mon, Apr 24, 2017 at 07:54:18PM +0800, Jason Wang wrote:

On 2017年04月24日 07:28, Michael S. Tsirkin wrote:

On Tue, Apr 18, 2017 at 11:07:42AM +0800, Jason Wang wrote:

On 2017年04月17日 07:19, Michael S. Tsirkin wrote:

Applications that consume a batch of entries in one go
can benefit from ability to return some of them back
into the ring.

Add an API for that - assuming there's space. If there's no space
naturally we can't do this and have to drop entries, but this implies
ring is full so we'd likely drop some anyway.

Signed-off-by: Michael S. Tsirkin
---

Jason, in my mind the biggest issue with your batching patchset is the
backet drops on disconnect.  This API will help avoid that in the common
case.

Ok, I will rebase the series on top of this. (Though I don't think we care
the packet loss).

E.g. I care - I often start sending packets to VM before it's
fully booted. Several vhost resets might follow.

Ok.


I would still prefer that we understand what's going on,

I try to reply in another thread, does it make sense?


and I would
like to know what's the smallest batch size that's still helpful,

Yes, I've replied in another thread, the result is:


no batching   1.88Mpps
RX_BATCH=11.93Mpps
RX_BATCH=42.11Mpps
RX_BATCH=16   2.14Mpps
RX_BATCH=64   2.25Mpps
RX_BATCH=256  2.18Mpps

Essentially 4 is enough, other stuf looks more like noise
to me. What about 2?

The numbers are pretty stable, so probably not noise. Retested on top of
batch zeroing:

no  1.97Mpps
1   2.09Mpps
2   2.11Mpps
4   2.16Mpps
8   2.19Mpps
16  2.21Mpps
32  2.25Mpps
64  2.30Mpps
128 2.21Mpps
256 2.21Mpps

64 performs best.

Thanks

OK but it might be e.g. a function of the ring size, host cache size or
whatever. As we don't really understand the why, if we just optimize for
your setup we risk regressions in others.  64 entries is a lot, it
increases the queue size noticeably.  Could this be part of the effect?
Could you try changing the queue size to see what happens?


I increase tx_queue_len to 1100, but only see less than 1% improvement 
on pps number (batch = 1) in my machine. If you care about the 
regression, we probably can leave the choice to user through e.g module 
parameter. But I'm afraid we have already had too much choices for them. 
Or I can test this with different CPU types.


Thanks

Re: [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps)

[Jens Axboe]

On Fri, Apr 21 2017, Andy Lutomirski wrote:
> Hi all-
> 
> These are APST improvements for 4.12 or so.  The first one fixes a
> buggy comment.  The second makes debugging easier.  The third makes
> it possible to force APST on despite quirks.
> 
> Andy Lutomirski (3):
>   nvme: Fix APST comment
>   nvme: Display raw APST configuration via DYNAMIC_DEBUG
>   nvme: Add nvme_core.force_apst to ignore the NO_APST quirk
> 
>  drivers/nvme/host/core.c | 38 --
>  1 file changed, 36 insertions(+), 2 deletions(-)

I have added these for 4.12, thanks Andy.

-- 
Jens Axboe

Re: [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps)

[Jens Axboe]

On Fri, Apr 21 2017, Andy Lutomirski wrote:
> Hi all-
> 
> These are APST improvements for 4.12 or so.  The first one fixes a
> buggy comment.  The second makes debugging easier.  The third makes
> it possible to force APST on despite quirks.
> 
> Andy Lutomirski (3):
>   nvme: Fix APST comment
>   nvme: Display raw APST configuration via DYNAMIC_DEBUG
>   nvme: Add nvme_core.force_apst to ignore the NO_APST quirk
> 
>  drivers/nvme/host/core.c | 38 --
>  1 file changed, 36 insertions(+), 2 deletions(-)

I have added these for 4.12, thanks Andy.

-- 
Jens Axboe

Re: [PATCH 1/3 v2] xen: Export xen_reboot

[Juergen Gross]

On 24/04/17 20:21, Boris Ostrovsky wrote:
> On 04/24/2017 01:58 PM, Julien Grall wrote:
>> The helper xen_reboot will be called by the EFI code in a later patch.
>>
>> Note that the ARM version does not yet exist and will be added in a
>> later patch too.
>>
>> Signed-off-by: Julien Grall 
> 
> I don't think these changes are worth a whole patch. They can be folded
> into the third patch.

No, the 2nd patch needs this, too.

So:

Reviewed-by: Juergen Gross 


Thanks, Juergen

Re: [PATCH 1/3 v2] xen: Export xen_reboot

[Juergen Gross]

On 24/04/17 20:21, Boris Ostrovsky wrote:
> On 04/24/2017 01:58 PM, Julien Grall wrote:
>> The helper xen_reboot will be called by the EFI code in a later patch.
>>
>> Note that the ARM version does not yet exist and will be added in a
>> later patch too.
>>
>> Signed-off-by: Julien Grall 
> 
> I don't think these changes are worth a whole patch. They can be folded
> into the third patch.

No, the 2nd patch needs this, too.

So:

Reviewed-by: Juergen Gross 


Thanks, Juergen

Re: [PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

[Vinod Koul]

On Tue, Apr 25, 2017 at 03:28:17AM +, Wei Yongjun wrote:
> From: Wei Yongjun 
> 
> Since we will remove items off the list using list_del() we need
> to use a safe version of the list_for_each() macro aptly named
> list_for_each_safe().

ah yes, god catch

> This is detected by Coccinelle semantic patch.

It is a good practice to include the script you used, do you mind adding
that in the log?

> 
> Fixes: b8c722ddd548 ("ASoC: Intel: Skylake: Add support for deferred
> DSP module bind")
> Signed-off-by: Wei Yongjun 
> ---
>  sound/soc/intel/skylake/skl-pcm.c | 4 ++--
>  sound/soc/intel/skylake/skl-topology.c | 4 ++--
>  2 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/intel/skylake/skl-pcm.c 
> b/sound/soc/intel/skylake/skl-pcm.c
> index 600faad..d43d197 100644
> --- a/sound/soc/intel/skylake/skl-pcm.c
> +++ b/sound/soc/intel/skylake/skl-pcm.c
> @@ -1323,10 +1323,10 @@ int skl_platform_unregister(struct device *dev)
>  {
>   struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
>   struct skl *skl = ebus_to_skl(ebus);
> - struct skl_module_deferred_bind *modules;
> + struct skl_module_deferred_bind *modules, *tmp;
>  
>   if (!list_empty(>bind_list)) {
> - list_for_each_entry(modules, >bind_list, node) {
> + list_for_each_entry_safe(modules, tmp, >bind_list, node) {
>   list_del(>node);
>   kfree(modules);
>   }
> diff --git a/sound/soc/intel/skylake/skl-topology.c 
> b/sound/soc/intel/skylake/skl-topology.c
> index 74b3acf..aea7917 100644
> --- a/sound/soc/intel/skylake/skl-topology.c
> +++ b/sound/soc/intel/skylake/skl-topology.c
> @@ -1091,7 +1091,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct 
> snd_soc_dapm_widget *w,
>   struct skl_module_cfg *src_module = NULL, *dst_module;
>   struct skl_sst *ctx = skl->skl_sst;
>   struct skl_pipe *s_pipe = mconfig->pipe;
> - struct skl_module_deferred_bind *modules;
> + struct skl_module_deferred_bind *modules, *tmp;
>  
>   if (s_pipe->state == SKL_PIPE_INVALID)
>   return -EINVAL;
> @@ -1105,7 +1105,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct 
> snd_soc_dapm_widget *w,
>  
>   src_module = w_module->w->priv;
>  
> - list_for_each_entry(modules, >bind_list, node) {
> + list_for_each_entry_safe(modules, tmp, >bind_list, node) {
>   /*
>* When the destination module is deleted, Unbind the
>* modules from deferred bind list.
> 

-- 
~Vinod

Re: [PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

[Vinod Koul]

On Tue, Apr 25, 2017 at 03:28:17AM +, Wei Yongjun wrote:
> From: Wei Yongjun 
> 
> Since we will remove items off the list using list_del() we need
> to use a safe version of the list_for_each() macro aptly named
> list_for_each_safe().

ah yes, god catch

> This is detected by Coccinelle semantic patch.

It is a good practice to include the script you used, do you mind adding
that in the log?

> 
> Fixes: b8c722ddd548 ("ASoC: Intel: Skylake: Add support for deferred
> DSP module bind")
> Signed-off-by: Wei Yongjun 
> ---
>  sound/soc/intel/skylake/skl-pcm.c | 4 ++--
>  sound/soc/intel/skylake/skl-topology.c | 4 ++--
>  2 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/intel/skylake/skl-pcm.c 
> b/sound/soc/intel/skylake/skl-pcm.c
> index 600faad..d43d197 100644
> --- a/sound/soc/intel/skylake/skl-pcm.c
> +++ b/sound/soc/intel/skylake/skl-pcm.c
> @@ -1323,10 +1323,10 @@ int skl_platform_unregister(struct device *dev)
>  {
>   struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
>   struct skl *skl = ebus_to_skl(ebus);
> - struct skl_module_deferred_bind *modules;
> + struct skl_module_deferred_bind *modules, *tmp;
>  
>   if (!list_empty(>bind_list)) {
> - list_for_each_entry(modules, >bind_list, node) {
> + list_for_each_entry_safe(modules, tmp, >bind_list, node) {
>   list_del(>node);
>   kfree(modules);
>   }
> diff --git a/sound/soc/intel/skylake/skl-topology.c 
> b/sound/soc/intel/skylake/skl-topology.c
> index 74b3acf..aea7917 100644
> --- a/sound/soc/intel/skylake/skl-topology.c
> +++ b/sound/soc/intel/skylake/skl-topology.c
> @@ -1091,7 +1091,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct 
> snd_soc_dapm_widget *w,
>   struct skl_module_cfg *src_module = NULL, *dst_module;
>   struct skl_sst *ctx = skl->skl_sst;
>   struct skl_pipe *s_pipe = mconfig->pipe;
> - struct skl_module_deferred_bind *modules;
> + struct skl_module_deferred_bind *modules, *tmp;
>  
>   if (s_pipe->state == SKL_PIPE_INVALID)
>   return -EINVAL;
> @@ -1105,7 +1105,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct 
> snd_soc_dapm_widget *w,
>  
>   src_module = w_module->w->priv;
>  
> - list_for_each_entry(modules, >bind_list, node) {
> + list_for_each_entry_safe(modules, tmp, >bind_list, node) {
>   /*
>* When the destination module is deleted, Unbind the
>* modules from deferred bind list.
> 

-- 
~Vinod

Re: [PATCH v2 02/29] drm/ttm: fix include notation and remove -Iinclude/drm flag

[Michel Dänzer]

On 24/04/17 01:50 PM, Masahiro Yamada wrote:
> For the C file, include  instead of relative path from
> include/drm.
> 
> For headers in include/drm/ttm, simplify the  with "*.h".
> 
> This allows us to remove the -Iinclude/drm compiler flag from
> drivers/gpu/drm/ttm/Makefile (and from other drivers' Makefiles).
> 
> Signed-off-by: Masahiro Yamada 
> ---
> 
> Changes in v2:
>   - Use #include "..." for headers  (Michel Danzer)

Thanks!

Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

Re: [PATCH v2 02/29] drm/ttm: fix include notation and remove -Iinclude/drm flag

[Michel Dänzer]

On 24/04/17 01:50 PM, Masahiro Yamada wrote:
> For the C file, include  instead of relative path from
> include/drm.
> 
> For headers in include/drm/ttm, simplify the  with "*.h".
> 
> This allows us to remove the -Iinclude/drm compiler flag from
> drivers/gpu/drm/ttm/Makefile (and from other drivers' Makefiles).
> 
> Signed-off-by: Masahiro Yamada 
> ---
> 
> Changes in v2:
>   - Use #include "..." for headers  (Michel Danzer)

Thanks!

Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

Re: [PATCH] ARM: dts: Add devicetree for the Raspberry Pi 3, for arm32 (v5)

[kbuild test robot]

Hi Eric,

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

url:
https://github.com/0day-ci/linux/commits/Eric-Anholt/ARM-dts-Add-devicetree-for-the-Raspberry-Pi-3-for-arm32-v5/20170425-063618
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-at91_dt_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> arch/arm/boot/dts/bcm2837-rpi-3.b.dts:1:46: fatal error: 
>> arm64/broadcom/bcm2837-rpi-3.b.dts: No such file or directory
#include "arm64/broadcom/bcm2837-rpi-3.b.dts"
 ^
   compilation terminated.

vim +1 arch/arm/boot/dts/bcm2837-rpi-3.b.dts

   > 1  #include "arm64/broadcom/bcm2837-rpi-3.b.dts"

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] ARM: dts: Add devicetree for the Raspberry Pi 3, for arm32 (v5)

[kbuild test robot]

Hi Eric,

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

url:
https://github.com/0day-ci/linux/commits/Eric-Anholt/ARM-dts-Add-devicetree-for-the-Raspberry-Pi-3-for-arm32-v5/20170425-063618
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-at91_dt_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> arch/arm/boot/dts/bcm2837-rpi-3.b.dts:1:46: fatal error: 
>> arm64/broadcom/bcm2837-rpi-3.b.dts: No such file or directory
#include "arm64/broadcom/bcm2837-rpi-3.b.dts"
 ^
   compilation terminated.

vim +1 arch/arm/boot/dts/bcm2837-rpi-3.b.dts

   > 1  #include "arm64/broadcom/bcm2837-rpi-3.b.dts"

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] sched: topology: s/borken/broken

[Viresh Kumar]

On 24-04-17, 14:26, Mike Galbraith wrote:
> On Mon, 2017-04-24 at 16:50 +0530, Viresh Kumar wrote:
> > Fix minor spelling mistake.
> 
> That's a perfectly correct alternative spelling of b0rken :)

yeah, I was already trolled on IRC for this stupid patch. Sorry about that and
this one is dropped for ever.

-- 
viresh

Re: [PATCH] sched: topology: s/borken/broken

[Viresh Kumar]

On 24-04-17, 14:26, Mike Galbraith wrote:
> On Mon, 2017-04-24 at 16:50 +0530, Viresh Kumar wrote:
> > Fix minor spelling mistake.
> 
> That's a perfectly correct alternative spelling of b0rken :)

yeah, I was already trolled on IRC for this stupid patch. Sorry about that and
this one is dropped for ever.

-- 
viresh

Re: [PATCH v2] arm64: perf: Use only exclude_kernel attribute when kernel is running in HYP

[Ganapatrao Kulkarni]

On Mon, Apr 24, 2017 at 9:15 PM, Will Deacon  wrote:
> On Thu, Apr 20, 2017 at 02:56:50PM +0530, Ganapatrao Kulkarni wrote:
>> On Thu, Apr 20, 2017 at 2:19 PM, Mark Rutland  wrote:
>> > On Wed, Apr 19, 2017 at 11:14:06PM +0530, Ganapatrao Kulkarni wrote:
>> >> commit d98ecda (arm64: perf: Count EL2 events if the kernel is running in 
>> >> HYP)
>> >> is returning error for perf syscall with mixed attribute set for 
>> >> exclude_kernel
>> >> and exclude_hv. This change is breaking some applications (observed with 
>> >> hhvm)
>> >> when ran on VHE enabled platforms.
>> >>
>> >> Adding fix to consider only exclude_kernel attribute when kernel is
>> >> running in HYP. Also adding sysfs file to notify the bhehaviour
>> >> of attribute exclude_hv.
>> >>
>> >> Signed-off-by: Ganapatrao Kulkarni 
>> >> ---
>> >>
>> >> Changelog:
>> >>
>> >> V2:
>> >>  - Changes as per Will Deacon's suggestion.
>> >>
>> >> V1: Initial patch
>> >>
>> >>  arch/arm64/kernel/perf_event.c | 28 
>> >>  include/linux/perf/arm_pmu.h   |  1 +
>> >>  2 files changed, 25 insertions(+), 4 deletions(-)
>> >>
>> >> @@ -871,14 +890,13 @@ static int armv8pmu_set_event_filter(struct 
>> >> hw_perf_event *event,
>> >>
>> >>   if (attr->exclude_idle)
>> >>   return -EPERM;
>> >> - if (is_kernel_in_hyp_mode() &&
>> >> - attr->exclude_kernel != attr->exclude_hv)
>> >> - return -EINVAL;
>> >> + if (is_kernel_in_hyp_mode() && !attr->exclude_kernel)
>> >> + config_base |= ARMV8_PMU_INCLUDE_EL2;
>> >>   if (attr->exclude_user)
>> >>   config_base |= ARMV8_PMU_EXCLUDE_EL0;
>> >>   if (!is_kernel_in_hyp_mode() && attr->exclude_kernel)
>> >>   config_base |= ARMV8_PMU_EXCLUDE_EL1;
>> >> - if (!attr->exclude_hv)
>> >> + if (!is_kernel_in_hyp_mode() && !attr->exclude_hv)
>> >>   config_base |= ARMV8_PMU_INCLUDE_EL2;
>> >
>> > This isn't quite what Will suggested.
>> >
>> > The idea was that userspace would read sysfs, then use that to determine
>> > the correct exclusion parameters [1,2]. This logic was not expected to
>> > change; it correctly validates whether we can provide what the user
>> > requests.
>>
>> OK, if you are ok with sysfs part, i can send next version with that
>> change only?.
>
> I think the sysfs part is still a little dodgy, since you still expose the
> "exclude_hv" file with a value of 0 when not running at EL2, which would
> imply that exclude_hv is forced to zero. I don't think that's correct.

okay, i can make exclude_hv visible only when kernel booted in EL2.
is it ok to have empty directory "attr" when kernel booted to EL1?
attr can be place holder for any other miscellaneous attributes, that
can be added in future.

>
> Will

thanks
Ganapat

Re: [PATCH v2] arm64: perf: Use only exclude_kernel attribute when kernel is running in HYP

[Ganapatrao Kulkarni]

On Mon, Apr 24, 2017 at 9:15 PM, Will Deacon  wrote:
> On Thu, Apr 20, 2017 at 02:56:50PM +0530, Ganapatrao Kulkarni wrote:
>> On Thu, Apr 20, 2017 at 2:19 PM, Mark Rutland  wrote:
>> > On Wed, Apr 19, 2017 at 11:14:06PM +0530, Ganapatrao Kulkarni wrote:
>> >> commit d98ecda (arm64: perf: Count EL2 events if the kernel is running in 
>> >> HYP)
>> >> is returning error for perf syscall with mixed attribute set for 
>> >> exclude_kernel
>> >> and exclude_hv. This change is breaking some applications (observed with 
>> >> hhvm)
>> >> when ran on VHE enabled platforms.
>> >>
>> >> Adding fix to consider only exclude_kernel attribute when kernel is
>> >> running in HYP. Also adding sysfs file to notify the bhehaviour
>> >> of attribute exclude_hv.
>> >>
>> >> Signed-off-by: Ganapatrao Kulkarni 
>> >> ---
>> >>
>> >> Changelog:
>> >>
>> >> V2:
>> >>  - Changes as per Will Deacon's suggestion.
>> >>
>> >> V1: Initial patch
>> >>
>> >>  arch/arm64/kernel/perf_event.c | 28 
>> >>  include/linux/perf/arm_pmu.h   |  1 +
>> >>  2 files changed, 25 insertions(+), 4 deletions(-)
>> >>
>> >> @@ -871,14 +890,13 @@ static int armv8pmu_set_event_filter(struct 
>> >> hw_perf_event *event,
>> >>
>> >>   if (attr->exclude_idle)
>> >>   return -EPERM;
>> >> - if (is_kernel_in_hyp_mode() &&
>> >> - attr->exclude_kernel != attr->exclude_hv)
>> >> - return -EINVAL;
>> >> + if (is_kernel_in_hyp_mode() && !attr->exclude_kernel)
>> >> + config_base |= ARMV8_PMU_INCLUDE_EL2;
>> >>   if (attr->exclude_user)
>> >>   config_base |= ARMV8_PMU_EXCLUDE_EL0;
>> >>   if (!is_kernel_in_hyp_mode() && attr->exclude_kernel)
>> >>   config_base |= ARMV8_PMU_EXCLUDE_EL1;
>> >> - if (!attr->exclude_hv)
>> >> + if (!is_kernel_in_hyp_mode() && !attr->exclude_hv)
>> >>   config_base |= ARMV8_PMU_INCLUDE_EL2;
>> >
>> > This isn't quite what Will suggested.
>> >
>> > The idea was that userspace would read sysfs, then use that to determine
>> > the correct exclusion parameters [1,2]. This logic was not expected to
>> > change; it correctly validates whether we can provide what the user
>> > requests.
>>
>> OK, if you are ok with sysfs part, i can send next version with that
>> change only?.
>
> I think the sysfs part is still a little dodgy, since you still expose the
> "exclude_hv" file with a value of 0 when not running at EL2, which would
> imply that exclude_hv is forced to zero. I don't think that's correct.

okay, i can make exclude_hv visible only when kernel booted in EL2.
is it ok to have empty directory "attr" when kernel booted to EL1?
attr can be place holder for any other miscellaneous attributes, that
can be added in future.

>
> Will

thanks
Ganapat

Re: [PATCH v7 0/7] Introduce ZONE_CMA

[Joonsoo Kim]

On Mon, Apr 24, 2017 at 03:09:36PM +0200, Michal Hocko wrote:
> On Mon 17-04-17 11:02:12, Joonsoo Kim wrote:
> > On Thu, Apr 13, 2017 at 01:56:15PM +0200, Michal Hocko wrote:
> > > On Wed 12-04-17 10:35:06, Joonsoo Kim wrote:
> [...]
> > > > ZONE_CMA is conceptually the same with ZONE_MOVABLE. There is a software
> > > > constraint to guarantee the success of future allocation request from
> > > > the device. If the device requests the specific range of the memory in 
> > > > CMA
> > > > area at the runtime, page that allocated by MM will be migrated to
> > > > the other page and it will be returned to the device. To guarantee it,
> > > > ZONE_CMA only takes the allocation request with GFP_MOVABLE.
> > > 
> > > The immediate follow up question is. Why cannot we reuse ZONE_MOVABLE
> > > for that purpose?
> > 
> > I can make CMA reuses the ZONE_MOVABLE but I don't want it. Reasons
> > are that
> > 
> > 1. If ZONE_MOVABLE has two different types of memory, hotpluggable and
> > CMA, it may need special handling for each type. This would lead to a new
> > migratetype again (to distinguish them) and easy to be error-prone. I
> > don't want that case.
> 
> Hmm, I see your motivation. I believe that we could find a way
> around this. Anyway, movable zones are quite special and configuring
> overlapping CMA and hotplug movable regions could be refused. So I am
> not even sure this is a real problem in practice.
> 
> > 2. CMA users want to see usage stat separately since CMA often causes
> > the problems and separate stat would helps to debug it.
> 
> That could be solved by a per-zone/node counter.
> 
> Anyway, these reasons should be mentioned as well. Adding a new zone is

Okay.

> not for free. For most common configurations where we have ZONE_DMA,
> ZONE_DMA32, ZONE_NORMAL and ZONE_MOVABLE all the 3 bits are already
> consumed so a new zone will need a new one AFAICS.

Yes, it requires one more bit for a new zone and it's handled by the patch.

> 
> [...]
> > > > Other things are completely the same with other zones. For MM POV, 
> > > > there is
> > > > no difference in allocation process except that it only takes
> > > > GFP_MOVABLE request. In reclaim, pages that are allocated by MM will
> > > > be reclaimed by the same policy of the MM. So, no difference.
> > > 
> > > OK, so essentially this is yet another "highmem" zone. We already know
> > > that only GFP_MOVABLE are allowed to fallback to ZONE_CMA but do CMA
> > > allocations fallback to other zones and punch new holes? In which zone
> > > order?
> > 
> > Hmm... I don't understand your question. Could you elaborate it more?
> 
> Well, my question was about the zone fallback chain. MOVABLE allocation
> can fallback to lower zones and also to the ZONE_CMA with your patch. If
> there is a CMA allocation it doesn't fall back to any other zone - in
> other words no new holes are punched to other zones. Is this correct?

Hmm... I still don't get the meaning of "no new holes are punched to
other zones". I try to answer with my current understanding about your
question.

MOVABLE allocation will fallback as following sequence.

ZONE_CMA -> ZONE_MOVABLE -> ZONE_HIGHMEM -> ZONE_NORMAL -> ...

I don't understand what you mean CMA allocation. In MM's context,
there is no CMA allocation. That is just MOVABLE allocation.

For device's context, there is CMA allocation. It is range specific
allocation so it should be succeed for requested range. No fallback is
allowed in this case.

> > > > This 'no difference' is a strong point of this approach. ZONE_CMA is
> > > > naturally handled by MM subsystem unlike as before (special handling is
> > > > required for MIGRATE_CMA).
> > > > 
> > > > 3. Controversial Point
> > > > 
> > > > Major concern from Mel is that zone concept is abused. ZONE is 
> > > > originally
> > > > introduced to solve some issues due to H/W addressing limitation.
> > > 
> > > Yes, very much agreed on that. You basically want to punch holes into
> > > other zones to guarantee an allocation progress. Marking those wholes
> > > with special migrate type sounds quite natural but I will have to study
> > > the current code some more to see whether issues you mention are
> > > inherently unfixable. This might very well turn out to be the case.
> > 
> > At a glance, special migratetype sound natural. I also did. However,
> > it's not natural in implementation POV. Zone consists of the same type
> > of memory (by definition ?) and MM subsystem is implemented with that
> > assumption. If difference type of memory shares the same zone, it easily
> > causes the problem and CMA problems are the such case.
> 
> But this is not any different from the highmem vs. lowmem problems we
> already have, no? I have looked at your example in the cover where you
> mention utilization and the reclaim problems. With the node reclaim we
> will have pages from all zones on the same LRU(s). isolate_lru_pages
> will skip those from ZONE_CMA because their zone_idx is higher than
> 

Re: [PATCH v7 0/7] Introduce ZONE_CMA

[Joonsoo Kim]

On Mon, Apr 24, 2017 at 03:09:36PM +0200, Michal Hocko wrote:
> On Mon 17-04-17 11:02:12, Joonsoo Kim wrote:
> > On Thu, Apr 13, 2017 at 01:56:15PM +0200, Michal Hocko wrote:
> > > On Wed 12-04-17 10:35:06, Joonsoo Kim wrote:
> [...]
> > > > ZONE_CMA is conceptually the same with ZONE_MOVABLE. There is a software
> > > > constraint to guarantee the success of future allocation request from
> > > > the device. If the device requests the specific range of the memory in 
> > > > CMA
> > > > area at the runtime, page that allocated by MM will be migrated to
> > > > the other page and it will be returned to the device. To guarantee it,
> > > > ZONE_CMA only takes the allocation request with GFP_MOVABLE.
> > > 
> > > The immediate follow up question is. Why cannot we reuse ZONE_MOVABLE
> > > for that purpose?
> > 
> > I can make CMA reuses the ZONE_MOVABLE but I don't want it. Reasons
> > are that
> > 
> > 1. If ZONE_MOVABLE has two different types of memory, hotpluggable and
> > CMA, it may need special handling for each type. This would lead to a new
> > migratetype again (to distinguish them) and easy to be error-prone. I
> > don't want that case.
> 
> Hmm, I see your motivation. I believe that we could find a way
> around this. Anyway, movable zones are quite special and configuring
> overlapping CMA and hotplug movable regions could be refused. So I am
> not even sure this is a real problem in practice.
> 
> > 2. CMA users want to see usage stat separately since CMA often causes
> > the problems and separate stat would helps to debug it.
> 
> That could be solved by a per-zone/node counter.
> 
> Anyway, these reasons should be mentioned as well. Adding a new zone is

Okay.

> not for free. For most common configurations where we have ZONE_DMA,
> ZONE_DMA32, ZONE_NORMAL and ZONE_MOVABLE all the 3 bits are already
> consumed so a new zone will need a new one AFAICS.

Yes, it requires one more bit for a new zone and it's handled by the patch.

> 
> [...]
> > > > Other things are completely the same with other zones. For MM POV, 
> > > > there is
> > > > no difference in allocation process except that it only takes
> > > > GFP_MOVABLE request. In reclaim, pages that are allocated by MM will
> > > > be reclaimed by the same policy of the MM. So, no difference.
> > > 
> > > OK, so essentially this is yet another "highmem" zone. We already know
> > > that only GFP_MOVABLE are allowed to fallback to ZONE_CMA but do CMA
> > > allocations fallback to other zones and punch new holes? In which zone
> > > order?
> > 
> > Hmm... I don't understand your question. Could you elaborate it more?
> 
> Well, my question was about the zone fallback chain. MOVABLE allocation
> can fallback to lower zones and also to the ZONE_CMA with your patch. If
> there is a CMA allocation it doesn't fall back to any other zone - in
> other words no new holes are punched to other zones. Is this correct?

Hmm... I still don't get the meaning of "no new holes are punched to
other zones". I try to answer with my current understanding about your
question.

MOVABLE allocation will fallback as following sequence.

ZONE_CMA -> ZONE_MOVABLE -> ZONE_HIGHMEM -> ZONE_NORMAL -> ...

I don't understand what you mean CMA allocation. In MM's context,
there is no CMA allocation. That is just MOVABLE allocation.

For device's context, there is CMA allocation. It is range specific
allocation so it should be succeed for requested range. No fallback is
allowed in this case.

> > > > This 'no difference' is a strong point of this approach. ZONE_CMA is
> > > > naturally handled by MM subsystem unlike as before (special handling is
> > > > required for MIGRATE_CMA).
> > > > 
> > > > 3. Controversial Point
> > > > 
> > > > Major concern from Mel is that zone concept is abused. ZONE is 
> > > > originally
> > > > introduced to solve some issues due to H/W addressing limitation.
> > > 
> > > Yes, very much agreed on that. You basically want to punch holes into
> > > other zones to guarantee an allocation progress. Marking those wholes
> > > with special migrate type sounds quite natural but I will have to study
> > > the current code some more to see whether issues you mention are
> > > inherently unfixable. This might very well turn out to be the case.
> > 
> > At a glance, special migratetype sound natural. I also did. However,
> > it's not natural in implementation POV. Zone consists of the same type
> > of memory (by definition ?) and MM subsystem is implemented with that
> > assumption. If difference type of memory shares the same zone, it easily
> > causes the problem and CMA problems are the such case.
> 
> But this is not any different from the highmem vs. lowmem problems we
> already have, no? I have looked at your example in the cover where you
> mention utilization and the reclaim problems. With the node reclaim we
> will have pages from all zones on the same LRU(s). isolate_lru_pages
> will skip those from ZONE_CMA because their zone_idx is higher than
> 

Re: [PATCH RFC v6] x86,mm,sched: make lazy TLB mode even lazier

[Andy Lutomirski]

On Fri, Sep 9, 2016 at 12:44 AM, Peter Zijlstra  wrote:
> On Thu, Sep 08, 2016 at 09:39:45PM -0700, Andy Lutomirski wrote:
>> If they're busy threads, shouldn't the yield return immediately
>> because the threads are still ready to run?  Lazy TLB won't do much
>> unless you get the kernel in some state where it's running in the
>> context of a different kernel thread and hasn't switched to
>> swapper_pg_dir.  IIRC idle works like that, but you'd need to actually
>> sleep to go idle.
>
> Right, a task doing:
>
> for (;;) sched_yield();
>
> esp. when its the only runnable thread on the CPU, is a busy thread. It
> will not enter switch_mm(), which was where the invalidate hook was
> placed IIRC.

Hi all-

I'm guessing that this patch got abandoned, at least temporarily.  I'm
currently polishing up my PCID series, and I think it might be worth
revisiting this on top of my PCID rework.  The relevant major
infrastructure change I'm making with my PCID code is that I'm adding
an atomic64_t to each mm_context_t that gets incremented every time a
flush on that mm is requested.  With that change, we might be able to
get away with simply removing a cpu from mm_cpumask immediately when
it enters lazy mode and adding a hook to the scheduler to revalidate
the TLB state when switching mms when we were previously lazy.
Revalidation would just check that the counter hasn't changed.

--Andy

Re: [PATCH RFC v6] x86,mm,sched: make lazy TLB mode even lazier

[Andy Lutomirski]

On Fri, Sep 9, 2016 at 12:44 AM, Peter Zijlstra  wrote:
> On Thu, Sep 08, 2016 at 09:39:45PM -0700, Andy Lutomirski wrote:
>> If they're busy threads, shouldn't the yield return immediately
>> because the threads are still ready to run?  Lazy TLB won't do much
>> unless you get the kernel in some state where it's running in the
>> context of a different kernel thread and hasn't switched to
>> swapper_pg_dir.  IIRC idle works like that, but you'd need to actually
>> sleep to go idle.
>
> Right, a task doing:
>
> for (;;) sched_yield();
>
> esp. when its the only runnable thread on the CPU, is a busy thread. It
> will not enter switch_mm(), which was where the invalidate hook was
> placed IIRC.

Hi all-

I'm guessing that this patch got abandoned, at least temporarily.  I'm
currently polishing up my PCID series, and I think it might be worth
revisiting this on top of my PCID rework.  The relevant major
infrastructure change I'm making with my PCID code is that I'm adding
an atomic64_t to each mm_context_t that gets incremented every time a
flush on that mm is requested.  With that change, we might be able to
get away with simply removing a cpu from mm_cpumask immediately when
it enters lazy mode and adding a hook to the scheduler to revalidate
the TLB state when switching mms when we were previously lazy.
Revalidation would just check that the counter hasn't changed.

--Andy

[PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

[Wei Yongjun]

From: Wei Yongjun 

Since we will remove items off the list using list_del() we need
to use a safe version of the list_for_each() macro aptly named
list_for_each_safe().

This is detected by Coccinelle semantic patch.

Fixes: b8c722ddd548 ("ASoC: Intel: Skylake: Add support for deferred
DSP module bind")
Signed-off-by: Wei Yongjun 
---
 sound/soc/intel/skylake/skl-pcm.c | 4 ++--
 sound/soc/intel/skylake/skl-topology.c | 4 ++--
 2 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c 
b/sound/soc/intel/skylake/skl-pcm.c
index 600faad..d43d197 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1323,10 +1323,10 @@ int skl_platform_unregister(struct device *dev)
 {
struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
struct skl *skl = ebus_to_skl(ebus);
-   struct skl_module_deferred_bind *modules;
+   struct skl_module_deferred_bind *modules, *tmp;
 
if (!list_empty(>bind_list)) {
-   list_for_each_entry(modules, >bind_list, node) {
+   list_for_each_entry_safe(modules, tmp, >bind_list, node) {
list_del(>node);
kfree(modules);
}
diff --git a/sound/soc/intel/skylake/skl-topology.c 
b/sound/soc/intel/skylake/skl-topology.c
index 74b3acf..aea7917 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -1091,7 +1091,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct 
snd_soc_dapm_widget *w,
struct skl_module_cfg *src_module = NULL, *dst_module;
struct skl_sst *ctx = skl->skl_sst;
struct skl_pipe *s_pipe = mconfig->pipe;
-   struct skl_module_deferred_bind *modules;
+   struct skl_module_deferred_bind *modules, *tmp;
 
if (s_pipe->state == SKL_PIPE_INVALID)
return -EINVAL;
@@ -1105,7 +1105,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct 
snd_soc_dapm_widget *w,
 
src_module = w_module->w->priv;
 
-   list_for_each_entry(modules, >bind_list, node) {
+   list_for_each_entry_safe(modules, tmp, >bind_list, node) {
/*
 * When the destination module is deleted, Unbind the
 * modules from deferred bind list.

[PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

[Wei Yongjun]

From: Wei Yongjun 

Since we will remove items off the list using list_del() we need
to use a safe version of the list_for_each() macro aptly named
list_for_each_safe().

This is detected by Coccinelle semantic patch.

Fixes: b8c722ddd548 ("ASoC: Intel: Skylake: Add support for deferred
DSP module bind")
Signed-off-by: Wei Yongjun 
---
 sound/soc/intel/skylake/skl-pcm.c | 4 ++--
 sound/soc/intel/skylake/skl-topology.c | 4 ++--
 2 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c 
b/sound/soc/intel/skylake/skl-pcm.c
index 600faad..d43d197 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1323,10 +1323,10 @@ int skl_platform_unregister(struct device *dev)
 {
struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
struct skl *skl = ebus_to_skl(ebus);
-   struct skl_module_deferred_bind *modules;
+   struct skl_module_deferred_bind *modules, *tmp;
 
if (!list_empty(>bind_list)) {
-   list_for_each_entry(modules, >bind_list, node) {
+   list_for_each_entry_safe(modules, tmp, >bind_list, node) {
list_del(>node);
kfree(modules);
}
diff --git a/sound/soc/intel/skylake/skl-topology.c 
b/sound/soc/intel/skylake/skl-topology.c
index 74b3acf..aea7917 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -1091,7 +1091,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct 
snd_soc_dapm_widget *w,
struct skl_module_cfg *src_module = NULL, *dst_module;
struct skl_sst *ctx = skl->skl_sst;
struct skl_pipe *s_pipe = mconfig->pipe;
-   struct skl_module_deferred_bind *modules;
+   struct skl_module_deferred_bind *modules, *tmp;
 
if (s_pipe->state == SKL_PIPE_INVALID)
return -EINVAL;
@@ -1105,7 +1105,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct 
snd_soc_dapm_widget *w,
 
src_module = w_module->w->priv;
 
-   list_for_each_entry(modules, >bind_list, node) {
+   list_for_each_entry_safe(modules, tmp, >bind_list, node) {
/*
 * When the destination module is deleted, Unbind the
 * modules from deferred bind list.

Re: [PATCH v3 3/7] kprobes: validate the symbol name length

[Masami Hiramatsu]

On Sun, 23 Apr 2017 15:44:32 +
"Naveen N. Rao"  wrote:

> >> >> +bool is_valid_kprobe_symbol_name(const char *name)
> >> > 
> >> > This just check the length of symbol_name buffer, and can contain
> >> > some invalid chars.
> >> 
> >> Yes, I kept the function name generic incase we would like to do more 
> >> validation in future, plus it's shorter than 
> >> is_valid_kprobe_symbol_name_len() ;-)
> > 
> > OK, if this is enough general, we'd better define this in
> > kernel/kallsyms.c or in kallsyms.h. Of course the function
> > should be called is_valid_symbol_name(). :-)
> 
> I actually think this should be done in kprobes itself. The primary 
> intent is to perform such validation right when we first obtain the 
> input from the user. In this case, however, kallsyms_lookup_name() is 
> also an exported symbol, so I do think some validation there would be 
> good to have as well.

IMHO, it is natural that kallsyms will know what is valid symbols.
Providing validation function by kprobes means kprobes also knows
that, and I concerns that may lead a double standard.

Thanks,

> >> >> +{
> >> >> +   size_t sym_len;
> >> >> +   char *s;
> >> >> +
> >> >> +   s = strchr(name, ':');
> >> 
> >> Hmm.. this should be strnchr(). I re-factored the code that moved the 
> >> strnlen() above this below. I'll fix this.
> >> 
> >> >> +   if (s) {
> >> >> +   sym_len = strnlen(s+1, KSYM_NAME_LEN);
> >> > 
> >> > If you use strnlen() here, you just need to ensure sym_len < 
> >> > KSYM_NAME_LEN.
> >> 
> >> Hmm.. not sure I follow. Are you saying the check for sym_len <= 0 is 
> >> not needed?
> > 
> > You can check sym_len != 0, but anyway, here we concern about
> > "longer" string (for performance reason), we can focus on
> > such case.
> > (BTW, could you also check the name != NULL at first?)
> > 
> > So, what I think it can be;
> > 
> > if (strnlen(s+1, KSYM_NAME_LEN) == KSYM_NAME_LEN ||
> > (size_t)(s - name) >= MODULE_NAME_LEN)
> > return false;
> 
> Sure, thanks. I clearly need to refactor this code better!
> 
> - Naveen
> 
> 


-- 
Masami Hiramatsu 

Re: [PATCH v3 3/7] kprobes: validate the symbol name length

[Masami Hiramatsu]

On Sun, 23 Apr 2017 15:44:32 +
"Naveen N. Rao"  wrote:

> >> >> +bool is_valid_kprobe_symbol_name(const char *name)
> >> > 
> >> > This just check the length of symbol_name buffer, and can contain
> >> > some invalid chars.
> >> 
> >> Yes, I kept the function name generic incase we would like to do more 
> >> validation in future, plus it's shorter than 
> >> is_valid_kprobe_symbol_name_len() ;-)
> > 
> > OK, if this is enough general, we'd better define this in
> > kernel/kallsyms.c or in kallsyms.h. Of course the function
> > should be called is_valid_symbol_name(). :-)
> 
> I actually think this should be done in kprobes itself. The primary 
> intent is to perform such validation right when we first obtain the 
> input from the user. In this case, however, kallsyms_lookup_name() is 
> also an exported symbol, so I do think some validation there would be 
> good to have as well.

IMHO, it is natural that kallsyms will know what is valid symbols.
Providing validation function by kprobes means kprobes also knows
that, and I concerns that may lead a double standard.

Thanks,

> >> >> +{
> >> >> +   size_t sym_len;
> >> >> +   char *s;
> >> >> +
> >> >> +   s = strchr(name, ':');
> >> 
> >> Hmm.. this should be strnchr(). I re-factored the code that moved the 
> >> strnlen() above this below. I'll fix this.
> >> 
> >> >> +   if (s) {
> >> >> +   sym_len = strnlen(s+1, KSYM_NAME_LEN);
> >> > 
> >> > If you use strnlen() here, you just need to ensure sym_len < 
> >> > KSYM_NAME_LEN.
> >> 
> >> Hmm.. not sure I follow. Are you saying the check for sym_len <= 0 is 
> >> not needed?
> > 
> > You can check sym_len != 0, but anyway, here we concern about
> > "longer" string (for performance reason), we can focus on
> > such case.
> > (BTW, could you also check the name != NULL at first?)
> > 
> > So, what I think it can be;
> > 
> > if (strnlen(s+1, KSYM_NAME_LEN) == KSYM_NAME_LEN ||
> > (size_t)(s - name) >= MODULE_NAME_LEN)
> > return false;
> 
> Sure, thanks. I clearly need to refactor this code better!
> 
> - Naveen
> 
> 


-- 
Masami Hiramatsu 

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

[Michel Dänzer]

On 24/04/17 03:25 PM, Gerd Hoffmann wrote:
> Return correct fourcc codes on bigendian.  Drivers must be adapted to
> this change.
> 
> Signed-off-by: Gerd Hoffmann 

Just to reiterate, this won't work for the radeon driver, which programs
the GPU to use (effectively, per the current definition that these are
little endian GPU formats) DRM_FORMAT_XRGB with pre-R600 and
DRM_FORMAT_BGRX with >= R600.


> +#ifdef __BIG_ENDIAN
> + switch (bpp) {
> + case 8:
> + fmt = DRM_FORMAT_C8;
> + break;
> + case 24:
> + fmt = DRM_FORMAT_BGR888;
> + break;

BTW, endianness as a concept cannot apply to 8 or 24 bpp formats.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

[Michel Dänzer]

On 24/04/17 03:25 PM, Gerd Hoffmann wrote:
> Return correct fourcc codes on bigendian.  Drivers must be adapted to
> this change.
> 
> Signed-off-by: Gerd Hoffmann 

Just to reiterate, this won't work for the radeon driver, which programs
the GPU to use (effectively, per the current definition that these are
little endian GPU formats) DRM_FORMAT_XRGB with pre-R600 and
DRM_FORMAT_BGRX with >= R600.


> +#ifdef __BIG_ENDIAN
> + switch (bpp) {
> + case 8:
> + fmt = DRM_FORMAT_C8;
> + break;
> + case 24:
> + fmt = DRM_FORMAT_BGR888;
> + break;

BTW, endianness as a concept cannot apply to 8 or 24 bpp formats.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

[PATCH -next] drm/rockchip: cdn-dp: fix return value check in cdn_dp_probe()

[Wei Yongjun]

From: Wei Yongjun 

Fix the retrn value check which testing the wrong variable
in cdn_dp_probe().

Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399")
Signed-off-by: Wei Yongjun 
---
 drivers/gpu/drm/rockchip/cdn-dp-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c 
b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index 9ab67a6..6a4a113 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -1198,7 +1198,7 @@ static int cdn_dp_probe(struct platform_device *pdev)
continue;
 
port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
-   if (!dp)
+   if (!port)
return -ENOMEM;
 
port->extcon = extcon;

[PATCH -next] drm/rockchip: cdn-dp: fix return value check in cdn_dp_probe()

[Wei Yongjun]

From: Wei Yongjun 

Fix the retrn value check which testing the wrong variable
in cdn_dp_probe().

Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399")
Signed-off-by: Wei Yongjun 
---
 drivers/gpu/drm/rockchip/cdn-dp-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c 
b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index 9ab67a6..6a4a113 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -1198,7 +1198,7 @@ static int cdn_dp_probe(struct platform_device *pdev)
continue;
 
port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
-   if (!dp)
+   if (!port)
return -ENOMEM;
 
port->extcon = extcon;

[PATCH -next] phy: qcom-qmp: fix return value check in qcom_qmp_phy_create()

[Wei Yongjun]

From: Wei Yongjun 

In case of error, the function of_iomap() returns NULL pointer
not ERR_PTR(). The IS_ERR() test in the return value check should
be replaced with NULL test.

Fixes: e78f3d15e115 ("phy: qcom-qmp: new qmp phy driver for qcom-chipsets")
Signed-off-by: Wei Yongjun 
---
 drivers/phy/phy-qcom-qmp.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/phy/phy-qcom-qmp.c b/drivers/phy/phy-qcom-qmp.c
index 727e23b..a25c29d 100644
--- a/drivers/phy/phy-qcom-qmp.c
+++ b/drivers/phy/phy-qcom-qmp.c
@@ -983,16 +983,16 @@ int qcom_qmp_phy_create(struct device *dev, struct 
device_node *np, int id)
 * Resources are indexed as: tx -> 0; rx -> 1; pcs -> 2.
 */
qphy->tx = of_iomap(np, 0);
-   if (IS_ERR(qphy->tx))
-   return PTR_ERR(qphy->tx);
+   if (!qphy->tx)
+   return -ENOMEM;
 
qphy->rx = of_iomap(np, 1);
-   if (IS_ERR(qphy->rx))
-   return PTR_ERR(qphy->rx);
+   if (!qphy->rx)
+   return -ENOMEM;
 
qphy->pcs = of_iomap(np, 2);
-   if (IS_ERR(qphy->pcs))
-   return PTR_ERR(qphy->pcs);
+   if (!qphy->pcs)
+   return -ENOMEM;
 
/*
 * Get PHY's Pipe clock, if any. USB3 and PCIe are PIPE3

[PATCH -next] phy: qcom-qmp: fix return value check in qcom_qmp_phy_create()

[Wei Yongjun]

From: Wei Yongjun 

In case of error, the function of_iomap() returns NULL pointer
not ERR_PTR(). The IS_ERR() test in the return value check should
be replaced with NULL test.

Fixes: e78f3d15e115 ("phy: qcom-qmp: new qmp phy driver for qcom-chipsets")
Signed-off-by: Wei Yongjun 
---
 drivers/phy/phy-qcom-qmp.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/phy/phy-qcom-qmp.c b/drivers/phy/phy-qcom-qmp.c
index 727e23b..a25c29d 100644
--- a/drivers/phy/phy-qcom-qmp.c
+++ b/drivers/phy/phy-qcom-qmp.c
@@ -983,16 +983,16 @@ int qcom_qmp_phy_create(struct device *dev, struct 
device_node *np, int id)
 * Resources are indexed as: tx -> 0; rx -> 1; pcs -> 2.
 */
qphy->tx = of_iomap(np, 0);
-   if (IS_ERR(qphy->tx))
-   return PTR_ERR(qphy->tx);
+   if (!qphy->tx)
+   return -ENOMEM;
 
qphy->rx = of_iomap(np, 1);
-   if (IS_ERR(qphy->rx))
-   return PTR_ERR(qphy->rx);
+   if (!qphy->rx)
+   return -ENOMEM;
 
qphy->pcs = of_iomap(np, 2);
-   if (IS_ERR(qphy->pcs))
-   return PTR_ERR(qphy->pcs);
+   if (!qphy->pcs)
+   return -ENOMEM;
 
/*
 * Get PHY's Pipe clock, if any. USB3 and PCIe are PIPE3

[PATCH -next] firmware: google memconsole: Fix return value check in platform_memconsole_init()

[Wei Yongjun]

From: Wei Yongjun 

In case of error, the function platform_device_register_simple() returns
ERR_PTR() and never returns NULL. The NULL test in the return value
check should be replaced with IS_ERR().

Fixes: d384d6f43d1e ("firmware: google memconsole: Add coreboot support")
Signed-off-by: Wei Yongjun 
---
 drivers/firmware/google/memconsole-coreboot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/google/memconsole-coreboot.c 
b/drivers/firmware/google/memconsole-coreboot.c
index 2121014..027 100644
--- a/drivers/firmware/google/memconsole-coreboot.c
+++ b/drivers/firmware/google/memconsole-coreboot.c
@@ -95,8 +95,8 @@ static int __init platform_memconsole_init(void)
struct platform_device *pdev;
 
pdev = platform_device_register_simple("memconsole", -1, NULL, 0);
-   if (pdev == NULL)
-   return -ENODEV;
+   if (IS_ERR(pdev))
+   return PTR_ERR(pdev);
 
platform_driver_register(_driver);

[PATCH -next] firmware: google memconsole: Fix return value check in platform_memconsole_init()

[Wei Yongjun]

From: Wei Yongjun 

In case of error, the function platform_device_register_simple() returns
ERR_PTR() and never returns NULL. The NULL test in the return value
check should be replaced with IS_ERR().

Fixes: d384d6f43d1e ("firmware: google memconsole: Add coreboot support")
Signed-off-by: Wei Yongjun 
---
 drivers/firmware/google/memconsole-coreboot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/google/memconsole-coreboot.c 
b/drivers/firmware/google/memconsole-coreboot.c
index 2121014..027 100644
--- a/drivers/firmware/google/memconsole-coreboot.c
+++ b/drivers/firmware/google/memconsole-coreboot.c
@@ -95,8 +95,8 @@ static int __init platform_memconsole_init(void)
struct platform_device *pdev;
 
pdev = platform_device_register_simple("memconsole", -1, NULL, 0);
-   if (pdev == NULL)
-   return -ENODEV;
+   if (IS_ERR(pdev))
+   return PTR_ERR(pdev);
 
platform_driver_register(_driver);

Re: [PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Michel Dänzer]

On 25/04/17 10:12 AM, Michel Dänzer wrote:
> On 24/04/17 10:03 PM, Ville Syrjälä wrote:
>> On Mon, Apr 24, 2017 at 03:57:02PM +0900, Michel Dänzer wrote:
>>> On 22/04/17 07:05 PM, Ville Syrjälä wrote:
 On Fri, Apr 21, 2017 at 06:14:31PM +0200, Gerd Hoffmann wrote:
>   Hi,
>
>>> My personal opinion is that formats in drm_fourcc.h should be 
>>> independent of the CPU byte order and the function 
>>> drm_mode_legacy_fb_format() and drivers depending on that incorrect 
>>> assumption be fixed instead.
>>
>> The problem is this isn't a kernel-internal thing any more.  With the
>> addition of the ADDFB2 ioctl the fourcc codes became part of the
>> kernel/userspace abi ...
>
> Ok, added some printk's to the ADDFB and ADDFB2 code paths and tested a
> bit.  Apparently pretty much all userspace still uses the ADDFB ioctl.
> xorg (modesetting driver) does.  gnome-shell in wayland mode does.
> Seems the big transition to ADDFB2 didn't happen yet.
>
> I guess that makes changing drm_mode_legacy_fb_format + drivers a
> reasonable option ...

 Yeah, I came to the same conclusion after chatting with some
 folks on irc.

 So my current idea is that we change any driver that wants to follow the
 CPU endianness
>>>
>>> This isn't really optional for various reasons, some of which have been
>>> covered in this discussion.
>>>
>>>
 to declare support for big endian formats if the CPU is
 big endian. Presumably these are mostly the virtual GPU drivers.

 Additonally we'll make the mapping performed by drm_mode_legacy_fb_format()
 driver controlled. That way drivers that got changed to follow CPU
 endianness can return a framebuffer that matches CPU endianness. And
 drivers that expect the GPU endianness to not depend on the CPU
 endianness will keep working as they do now. The downside is that users
 of the legacy addfb ioctl will need to magically know which endianness
 they will get, but that is apparently already the case. And users of
 addfb2 will keep on specifying the endianness explicitly with
 DRM_FORMAT_BIG_ENDIAN vs. 0.
>>>
>>> I'm afraid it's not that simple.
>>>
>>> The display hardware of older (pre-R600 generation) Radeon GPUs does not
>>> support the "big endian" formats directly. In order to allow userspace
>>> to access pixel data in native endianness with the CPU, we instead use
>>> byte-swapping functionality which only affects CPU access.
>>
>> OK, I'm getting confused. Based on our irc discussion I got the
>> impression you don't byte swap CPU accesses.
> 
> Sorry for the confusion. The radeon kernel driver does support
> byte-swapping for CPU access to VRAM with pre-R600 GPUs, and this is
> used for fbdev emulation. What I meant on IRC is that the xf86-video-ati
> radeon driver doesn't make use of this, mostly because it only applies
> while a BO is in VRAM, and userspace can't control when that's the case
> (while a BO isn't being scanned out).
> 
> 
>> But since you do, how do you deal with mixing 8bpp vs. 16bpp vs. 32bpp?
> 
> The byte-swapping is configured per-BO via the
> RADEON_TILING_SWAP_16/32BIT flags.

... which means that it's disabled by default, so it shouldn't affect
generic userspace. So exposing the GPU format directly should be
feasible in this case as well after all. Sorry for the noise. :(


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

Re: [PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Michel Dänzer]

On 25/04/17 10:12 AM, Michel Dänzer wrote:
> On 24/04/17 10:03 PM, Ville Syrjälä wrote:
>> On Mon, Apr 24, 2017 at 03:57:02PM +0900, Michel Dänzer wrote:
>>> On 22/04/17 07:05 PM, Ville Syrjälä wrote:
 On Fri, Apr 21, 2017 at 06:14:31PM +0200, Gerd Hoffmann wrote:
>   Hi,
>
>>> My personal opinion is that formats in drm_fourcc.h should be 
>>> independent of the CPU byte order and the function 
>>> drm_mode_legacy_fb_format() and drivers depending on that incorrect 
>>> assumption be fixed instead.
>>
>> The problem is this isn't a kernel-internal thing any more.  With the
>> addition of the ADDFB2 ioctl the fourcc codes became part of the
>> kernel/userspace abi ...
>
> Ok, added some printk's to the ADDFB and ADDFB2 code paths and tested a
> bit.  Apparently pretty much all userspace still uses the ADDFB ioctl.
> xorg (modesetting driver) does.  gnome-shell in wayland mode does.
> Seems the big transition to ADDFB2 didn't happen yet.
>
> I guess that makes changing drm_mode_legacy_fb_format + drivers a
> reasonable option ...

 Yeah, I came to the same conclusion after chatting with some
 folks on irc.

 So my current idea is that we change any driver that wants to follow the
 CPU endianness
>>>
>>> This isn't really optional for various reasons, some of which have been
>>> covered in this discussion.
>>>
>>>
 to declare support for big endian formats if the CPU is
 big endian. Presumably these are mostly the virtual GPU drivers.

 Additonally we'll make the mapping performed by drm_mode_legacy_fb_format()
 driver controlled. That way drivers that got changed to follow CPU
 endianness can return a framebuffer that matches CPU endianness. And
 drivers that expect the GPU endianness to not depend on the CPU
 endianness will keep working as they do now. The downside is that users
 of the legacy addfb ioctl will need to magically know which endianness
 they will get, but that is apparently already the case. And users of
 addfb2 will keep on specifying the endianness explicitly with
 DRM_FORMAT_BIG_ENDIAN vs. 0.
>>>
>>> I'm afraid it's not that simple.
>>>
>>> The display hardware of older (pre-R600 generation) Radeon GPUs does not
>>> support the "big endian" formats directly. In order to allow userspace
>>> to access pixel data in native endianness with the CPU, we instead use
>>> byte-swapping functionality which only affects CPU access.
>>
>> OK, I'm getting confused. Based on our irc discussion I got the
>> impression you don't byte swap CPU accesses.
> 
> Sorry for the confusion. The radeon kernel driver does support
> byte-swapping for CPU access to VRAM with pre-R600 GPUs, and this is
> used for fbdev emulation. What I meant on IRC is that the xf86-video-ati
> radeon driver doesn't make use of this, mostly because it only applies
> while a BO is in VRAM, and userspace can't control when that's the case
> (while a BO isn't being scanned out).
> 
> 
>> But since you do, how do you deal with mixing 8bpp vs. 16bpp vs. 32bpp?
> 
> The byte-swapping is configured per-BO via the
> RADEON_TILING_SWAP_16/32BIT flags.

... which means that it's disabled by default, so it shouldn't affect
generic userspace. So exposing the GPU format directly should be
feasible in this case as well after all. Sorry for the noise. :(


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

Re: [PATCH v3] selftests: ftrace: Allow some tests to be run in a tracing instance

[Masami Hiramatsu]

On Mon, 24 Apr 2017 18:01:38 -0400
Steven Rostedt  wrote:

> On Sun, 23 Apr 2017 09:02:24 +0900
> Masami Hiramatsu  wrote:
> 
> 
> > > >   
> > > > > +}
> > > > > +
> > > > >  eval_result() { # sigval
> > > > >case $1 in
> > > > >  $PASS)
> > > > > @@ -271,6 +275,21 @@ for t in $TEST_CASES; do
> > > > >run_test $t
> > > > >  done
> > > > >  
> > > > > +# Test on instance loop
> > > > > +FIRST_INSTANCE=0
> > > > > +for t in $TEST_CASES; do
> > > > > +  test_on_instance $t || continue
> > > > > +  if [ $FIRST_INSTANCE -eq 0 ]; then
> > > > > +FIRST_INSTANCE=1
> > > > > +echo "Running tests in a tracing instance:"
> > > > > +  fi
> > > > 
> > > > Ah, I see. This is important. And I would rather like to show
> > > > it on the description line of each test so that we can check
> > > > which test log is run in an instance. E.g. passing "(instance)"
> > > > message to run_test() and testcase() as the 2nd arg, and print
> > > > it in testlog and console?  
> > > 
> > > OK, I'll update to v4.  
> 
> What about something like this:

Ah, that's much better than I thought :)

Acked-by: Masami Hiramatsu 

Thank you!

> 
> -- Steve
> 
> diff --git a/tools/testing/selftests/ftrace/ftracetest 
> b/tools/testing/selftests/ftrace/ftracetest
> index a8631d9..32e6211 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -150,11 +150,16 @@ XFAILED_CASES=
>  UNDEFINED_CASES=
>  TOTAL_RESULT=0
>  
> +INSTANCE=
>  CASENO=0
>  testcase() { # testfile
>CASENO=$((CASENO+1))
>desc=`grep "^#[ \t]*description:" $1 | cut -f2 -d:`
> -  prlog -n "[$CASENO]$desc"
> +  prlog -n "[$CASENO]$INSTANCE$desc"
> +}
> +
> +test_on_instance() { # testfile
> +  grep -q "^#[ \t]*flags:.*instance" $1
>  }
>  
>  eval_result() { # sigval
> @@ -271,6 +276,17 @@ for t in $TEST_CASES; do
>run_test $t
>  done
>  
> +# Test on instance loop
> +INSTANCE=" (instance) "
> +for t in $TEST_CASES; do
> +  test_on_instance $t || continue
> +  SAVED_TRACING_DIR=$TRACING_DIR
> +  export TRACING_DIR=`mktemp -d $TRACING_DIR/instances/ftracetest.XX`
> +  run_test $t
> +  rmdir $TRACING_DIR
> +  TRACING_DIR=$SAVED_TRACING_DIR
> +done
> +
>  prlog ""
>  prlog "# of passed: " `echo $PASSED_CASES | wc -w`
>  prlog "# of failed: " `echo $FAILED_CASES | wc -w`


-- 
Masami Hiramatsu 

Re: [PATCH v3] selftests: ftrace: Allow some tests to be run in a tracing instance

[Masami Hiramatsu]

On Mon, 24 Apr 2017 18:01:38 -0400
Steven Rostedt  wrote:

> On Sun, 23 Apr 2017 09:02:24 +0900
> Masami Hiramatsu  wrote:
> 
> 
> > > >   
> > > > > +}
> > > > > +
> > > > >  eval_result() { # sigval
> > > > >case $1 in
> > > > >  $PASS)
> > > > > @@ -271,6 +275,21 @@ for t in $TEST_CASES; do
> > > > >run_test $t
> > > > >  done
> > > > >  
> > > > > +# Test on instance loop
> > > > > +FIRST_INSTANCE=0
> > > > > +for t in $TEST_CASES; do
> > > > > +  test_on_instance $t || continue
> > > > > +  if [ $FIRST_INSTANCE -eq 0 ]; then
> > > > > +FIRST_INSTANCE=1
> > > > > +echo "Running tests in a tracing instance:"
> > > > > +  fi
> > > > 
> > > > Ah, I see. This is important. And I would rather like to show
> > > > it on the description line of each test so that we can check
> > > > which test log is run in an instance. E.g. passing "(instance)"
> > > > message to run_test() and testcase() as the 2nd arg, and print
> > > > it in testlog and console?  
> > > 
> > > OK, I'll update to v4.  
> 
> What about something like this:

Ah, that's much better than I thought :)

Acked-by: Masami Hiramatsu 

Thank you!

> 
> -- Steve
> 
> diff --git a/tools/testing/selftests/ftrace/ftracetest 
> b/tools/testing/selftests/ftrace/ftracetest
> index a8631d9..32e6211 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -150,11 +150,16 @@ XFAILED_CASES=
>  UNDEFINED_CASES=
>  TOTAL_RESULT=0
>  
> +INSTANCE=
>  CASENO=0
>  testcase() { # testfile
>CASENO=$((CASENO+1))
>desc=`grep "^#[ \t]*description:" $1 | cut -f2 -d:`
> -  prlog -n "[$CASENO]$desc"
> +  prlog -n "[$CASENO]$INSTANCE$desc"
> +}
> +
> +test_on_instance() { # testfile
> +  grep -q "^#[ \t]*flags:.*instance" $1
>  }
>  
>  eval_result() { # sigval
> @@ -271,6 +276,17 @@ for t in $TEST_CASES; do
>run_test $t
>  done
>  
> +# Test on instance loop
> +INSTANCE=" (instance) "
> +for t in $TEST_CASES; do
> +  test_on_instance $t || continue
> +  SAVED_TRACING_DIR=$TRACING_DIR
> +  export TRACING_DIR=`mktemp -d $TRACING_DIR/instances/ftracetest.XX`
> +  run_test $t
> +  rmdir $TRACING_DIR
> +  TRACING_DIR=$SAVED_TRACING_DIR
> +done
> +
>  prlog ""
>  prlog "# of passed: " `echo $PASSED_CASES | wc -w`
>  prlog "# of failed: " `echo $FAILED_CASES | wc -w`


-- 
Masami Hiramatsu 

Re: [PATCH] led: ledtrig-transient: replace timer_list with hrtimer

[David Lin]

Hi Jacek,

On Mon, Apr 24, 2017 at 12:59 PM, Jacek Anaszewski
 wrote:
>
> Hi David,
>
> Thanks for the patch.
>
> Unfortunately we cannot switch to using hr timers just like that
> without introducing side effects for many devices. We had similar
> attempt of increasing timer tirgger accuracy two years ago [0].
>
> In short words, for drivers that can sleep while setting brightness
> and/or are using a bus like I2C you will not be able to enforce
> 1ms delay period.
>
> I recommend you to go through the thread [0] so that we had
> a well defined ground for the discussion on how to address this
> issue properly.
>

I think I understand the background now, and would agree that not all
the LED driver require hrtimer as human eye can't probably tell
there's a 10ms variation in a blink. However, there's a need to
support hrtimer if the LED subsystem claims support the use case of
vibrator (please see Documentation/leds/ledtrig-transient.txt) as even
a 5ms of variation is perceivable to the user. I'm thinking if a
better interim solution is to introduce a
LEDS_TRIGGER_TRANSIENT_HRTIMER config to work with both timers in
compile time. Would you agree?

David

Re: [PATCH] led: ledtrig-transient: replace timer_list with hrtimer

[David Lin]

Hi Jacek,

On Mon, Apr 24, 2017 at 12:59 PM, Jacek Anaszewski
 wrote:
>
> Hi David,
>
> Thanks for the patch.
>
> Unfortunately we cannot switch to using hr timers just like that
> without introducing side effects for many devices. We had similar
> attempt of increasing timer tirgger accuracy two years ago [0].
>
> In short words, for drivers that can sleep while setting brightness
> and/or are using a bus like I2C you will not be able to enforce
> 1ms delay period.
>
> I recommend you to go through the thread [0] so that we had
> a well defined ground for the discussion on how to address this
> issue properly.
>

I think I understand the background now, and would agree that not all
the LED driver require hrtimer as human eye can't probably tell
there's a 10ms variation in a blink. However, there's a need to
support hrtimer if the LED subsystem claims support the use case of
vibrator (please see Documentation/leds/ledtrig-transient.txt) as even
a 5ms of variation is perceivable to the user. I'm thinking if a
better interim solution is to introduce a
LEDS_TRIGGER_TRANSIENT_HRTIMER config to work with both timers in
compile time. Would you agree?

David

Re: [PATCH 20/22] tools arch: Sync arch/x86/lib/memcpy_64.S with the kernel

[Joe Perches]

On Mon, 2017-04-24 at 13:36 -0700, Luck, Tony wrote:
> On Mon, Apr 24, 2017 at 04:54:37PM -0300, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo 
> > 
> > Just a minor fix done in:
> > 
> >   Fixes: 26a37ab319a2 ("x86/mce: Fix copy/paste error in exception table 
> > entries")
> > 
> > Cc: Tony Luck 
> > Link: http://lkml.kernel.org/n/tip-ni9jzdd5yxlail6pq8cue...@git.kernel.org
> > Signed-off-by: Arnaldo Carvalho de Melo 
> > ---
> >  tools/arch/x86/lib/memcpy_64.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/arch/x86/lib/memcpy_64.S b/tools/arch/x86/lib/memcpy_64.S
> > index 49e6ebac7e73..98dcc112b363 100644
> > --- a/tools/arch/x86/lib/memcpy_64.S
> > +++ b/tools/arch/x86/lib/memcpy_64.S
> > @@ -286,7 +286,7 @@ ENDPROC(memcpy_mcsafe_unrolled)
> > _ASM_EXTABLE_FAULT(.L_copy_leading_bytes, .L_memcpy_mcsafe_fail)
> > _ASM_EXTABLE_FAULT(.L_cache_w0, .L_memcpy_mcsafe_fail)
> > _ASM_EXTABLE_FAULT(.L_cache_w1, .L_memcpy_mcsafe_fail)
> > -   _ASM_EXTABLE_FAULT(.L_cache_w3, .L_memcpy_mcsafe_fail)
> > +   _ASM_EXTABLE_FAULT(.L_cache_w2, .L_memcpy_mcsafe_fail)
> > _ASM_EXTABLE_FAULT(.L_cache_w3, .L_memcpy_mcsafe_fail)
> > _ASM_EXTABLE_FAULT(.L_cache_w4, .L_memcpy_mcsafe_fail)
> > _ASM_EXTABLE_FAULT(.L_cache_w5, .L_memcpy_mcsafe_fail)
> 
> If we are going to have all these copies of kernel files below
> "tools/...", perhaps checkpatch could warn people touching one
> that the other needs the same update?

How would checkpatch know tools hasn't already updated the other?

Re: [PATCH 20/22] tools arch: Sync arch/x86/lib/memcpy_64.S with the kernel

[Joe Perches]

On Mon, 2017-04-24 at 13:36 -0700, Luck, Tony wrote:
> On Mon, Apr 24, 2017 at 04:54:37PM -0300, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo 
> > 
> > Just a minor fix done in:
> > 
> >   Fixes: 26a37ab319a2 ("x86/mce: Fix copy/paste error in exception table 
> > entries")
> > 
> > Cc: Tony Luck 
> > Link: http://lkml.kernel.org/n/tip-ni9jzdd5yxlail6pq8cue...@git.kernel.org
> > Signed-off-by: Arnaldo Carvalho de Melo 
> > ---
> >  tools/arch/x86/lib/memcpy_64.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/arch/x86/lib/memcpy_64.S b/tools/arch/x86/lib/memcpy_64.S
> > index 49e6ebac7e73..98dcc112b363 100644
> > --- a/tools/arch/x86/lib/memcpy_64.S
> > +++ b/tools/arch/x86/lib/memcpy_64.S
> > @@ -286,7 +286,7 @@ ENDPROC(memcpy_mcsafe_unrolled)
> > _ASM_EXTABLE_FAULT(.L_copy_leading_bytes, .L_memcpy_mcsafe_fail)
> > _ASM_EXTABLE_FAULT(.L_cache_w0, .L_memcpy_mcsafe_fail)
> > _ASM_EXTABLE_FAULT(.L_cache_w1, .L_memcpy_mcsafe_fail)
> > -   _ASM_EXTABLE_FAULT(.L_cache_w3, .L_memcpy_mcsafe_fail)
> > +   _ASM_EXTABLE_FAULT(.L_cache_w2, .L_memcpy_mcsafe_fail)
> > _ASM_EXTABLE_FAULT(.L_cache_w3, .L_memcpy_mcsafe_fail)
> > _ASM_EXTABLE_FAULT(.L_cache_w4, .L_memcpy_mcsafe_fail)
> > _ASM_EXTABLE_FAULT(.L_cache_w5, .L_memcpy_mcsafe_fail)
> 
> If we are going to have all these copies of kernel files below
> "tools/...", perhaps checkpatch could warn people touching one
> that the other needs the same update?

How would checkpatch know tools hasn't already updated the other?

Re: your mail

[Joonsoo Kim]

On Mon, Apr 24, 2017 at 09:53:12AM +0200, Michal Hocko wrote:
> On Mon 24-04-17 10:44:43, Joonsoo Kim wrote:
> > On Fri, Apr 21, 2017 at 09:16:16AM +0200, Michal Hocko wrote:
> > > On Fri 21-04-17 13:38:28, Joonsoo Kim wrote:
> > > > On Thu, Apr 20, 2017 at 09:28:20AM +0200, Michal Hocko wrote:
> > > > > On Thu 20-04-17 10:27:55, Joonsoo Kim wrote:
> > > > > > On Mon, Apr 17, 2017 at 10:15:15AM +0200, Michal Hocko wrote:
> > > > > [...]
> > > > > > > Which pfn walkers you have in mind?
> > > > > > 
> > > > > > For example, kpagecount_read() in fs/proc/page.c. I searched it by
> > > > > > using pfn_valid().
> > > > > 
> > > > > Yeah, I've checked that one and in fact this is a good example of the
> > > > > case where you do not really care about holes. It just checks the page
> > > > > count which is a valid information under any circumstances.
> > > > 
> > > > I don't think so. First, it checks the page *map* count. Is it still 
> > > > valid
> > > > even if PageReserved() is set?
> > > 
> > > I do not know about any user which would manipulate page map count for
> > > referenced pages. The core MM code doesn't.
> > 
> > That's weird that we can get *map* count without PageReserved() check,
> > but we cannot get zone information.
> > Zone information is more static information than map count.
> 
> As I've already pointed out the rework of the hotplug code is mainly
> about postponing the zone initialization from the physical hot add to
> the logical onlining. The zone is really not clear until that moment.
>  
> > It should be defined/documented in this time that what information in
> > the struct page is valid even if PageReserved() is set. And then, we
> > need to fix all the things based on this design decision.
> 
> Where would you suggest documenting this? We do have
> Documentation/memory-hotplug.txt but it is not really specific about
> struct page.

pfn_valid() in include/linux/mmzone.h looks proper place.

> 
> [...]
> 
> > > You are trying to change a semantic of something that has a well defined
> > > meaning. I disagree that we should change it. It might sound like a
> > > simpler thing to do because pfn walkers will have to be checked but what
> > > you are proposing is conflating two different things together.
> > 
> > I don't think that *I* try to change the semantic of pfn_valid().
> > It would be original semantic of pfn_valid().
> > 
> > "If pfn_valid() returns true, we can get proper struct page and the
> > zone information,"
> 
> I do not see any guarantee about the zone information anywhere. In fact
> this is not true with the original implementation as I've tried to
> explain already. We do have new pages associated with a zone but that
> association might change during the online phase. So you cannot really
> rely on that information until the page is online. There is no real
> change in that regards after my rework.

I know that what you did doesn't change thing much. What I try to say
is that previous implementation related to pfn_valid() in hotplug is
wrong. Please do not assume that hotplug implementation is correct and
other pfn_valid() users are incorrect. There is no design document so
I'm not sure which one is correct but assumption that pfn_valid() user
can access whole the struct page information makes much sense to me.
So, I hope that please fix hotplug implementation rather than
modifying each pfn_valid() users.

> 
> [...]
> > > So please do not conflate those two different concepts together. I
> > > believe that the most prominent pfn walkers should be covered now and
> > > others can be evaluated later.
> > 
> > Even if original pfn_valid()'s semantic is not the one that I mentioned,
> > I think that suggested semantic from me is better.
> > Only hotplug code need to be changed and others doesn't need to be changed.
> > There is no overhead for others. What's the problem about this approach?
> 
> That this would require to check _every_ single pfn_valid user in the
> kernel. That is beyond my time capacity and not really necessary because
> the current code already suffers from the same/similar class of
> problems.

I think that all the pfn_valid() user doesn't consider hole case.
Unlike your expectation, if your way is taken, it requires to check
_every_ pfn_valid() users.

Thanks.

Re: your mail

[Joonsoo Kim]

On Mon, Apr 24, 2017 at 09:53:12AM +0200, Michal Hocko wrote:
> On Mon 24-04-17 10:44:43, Joonsoo Kim wrote:
> > On Fri, Apr 21, 2017 at 09:16:16AM +0200, Michal Hocko wrote:
> > > On Fri 21-04-17 13:38:28, Joonsoo Kim wrote:
> > > > On Thu, Apr 20, 2017 at 09:28:20AM +0200, Michal Hocko wrote:
> > > > > On Thu 20-04-17 10:27:55, Joonsoo Kim wrote:
> > > > > > On Mon, Apr 17, 2017 at 10:15:15AM +0200, Michal Hocko wrote:
> > > > > [...]
> > > > > > > Which pfn walkers you have in mind?
> > > > > > 
> > > > > > For example, kpagecount_read() in fs/proc/page.c. I searched it by
> > > > > > using pfn_valid().
> > > > > 
> > > > > Yeah, I've checked that one and in fact this is a good example of the
> > > > > case where you do not really care about holes. It just checks the page
> > > > > count which is a valid information under any circumstances.
> > > > 
> > > > I don't think so. First, it checks the page *map* count. Is it still 
> > > > valid
> > > > even if PageReserved() is set?
> > > 
> > > I do not know about any user which would manipulate page map count for
> > > referenced pages. The core MM code doesn't.
> > 
> > That's weird that we can get *map* count without PageReserved() check,
> > but we cannot get zone information.
> > Zone information is more static information than map count.
> 
> As I've already pointed out the rework of the hotplug code is mainly
> about postponing the zone initialization from the physical hot add to
> the logical onlining. The zone is really not clear until that moment.
>  
> > It should be defined/documented in this time that what information in
> > the struct page is valid even if PageReserved() is set. And then, we
> > need to fix all the things based on this design decision.
> 
> Where would you suggest documenting this? We do have
> Documentation/memory-hotplug.txt but it is not really specific about
> struct page.

pfn_valid() in include/linux/mmzone.h looks proper place.

> 
> [...]
> 
> > > You are trying to change a semantic of something that has a well defined
> > > meaning. I disagree that we should change it. It might sound like a
> > > simpler thing to do because pfn walkers will have to be checked but what
> > > you are proposing is conflating two different things together.
> > 
> > I don't think that *I* try to change the semantic of pfn_valid().
> > It would be original semantic of pfn_valid().
> > 
> > "If pfn_valid() returns true, we can get proper struct page and the
> > zone information,"
> 
> I do not see any guarantee about the zone information anywhere. In fact
> this is not true with the original implementation as I've tried to
> explain already. We do have new pages associated with a zone but that
> association might change during the online phase. So you cannot really
> rely on that information until the page is online. There is no real
> change in that regards after my rework.

I know that what you did doesn't change thing much. What I try to say
is that previous implementation related to pfn_valid() in hotplug is
wrong. Please do not assume that hotplug implementation is correct and
other pfn_valid() users are incorrect. There is no design document so
I'm not sure which one is correct but assumption that pfn_valid() user
can access whole the struct page information makes much sense to me.
So, I hope that please fix hotplug implementation rather than
modifying each pfn_valid() users.

> 
> [...]
> > > So please do not conflate those two different concepts together. I
> > > believe that the most prominent pfn walkers should be covered now and
> > > others can be evaluated later.
> > 
> > Even if original pfn_valid()'s semantic is not the one that I mentioned,
> > I think that suggested semantic from me is better.
> > Only hotplug code need to be changed and others doesn't need to be changed.
> > There is no overhead for others. What's the problem about this approach?
> 
> That this would require to check _every_ single pfn_valid user in the
> kernel. That is beyond my time capacity and not really necessary because
> the current code already suffers from the same/similar class of
> problems.

I think that all the pfn_valid() user doesn't consider hole case.
Unlike your expectation, if your way is taken, it requires to check
_every_ pfn_valid() users.

Thanks.

Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

[AceLan Kao]

According the spec. I have, the values are correct.
Please merge it, thanks.

2017-04-25 5:41 GMT+08:00 Maarten Maathuis :
> On Mon, Apr 24, 2017 at 11:37 PM, Andy Shevchenko
>  wrote:
>> On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis  
>> wrote:
>>> Tested on HP Elite X2 1012 G1.
>>> Matches event report of Lenovo Helix 2
>>> (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).
>>>
>>
>> Much better!
>>
>>> V2: Fix indent and add sign-off
>>
>> Usually this line goes after --- (body delimiter).
>> No need to resend this time. I would wait a bit for actual
>> author/driver maintainer to comment. Otherwise patch looks good enough
>> to me.
>
> The intent is not have this in the commit message?
> I'll keep an eye out if i can place it below "---" next time.
> Although i suspect the message would end in the actual code diff,
> which seems odd.
>
>>
>>>
>>> Signed-off-by: Maarten Maathuis 
>>> ---
>>>  drivers/platform/x86/intel-vbtn.c | 4 
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/intel-vbtn.c 
>>> b/drivers/platform/x86/intel-vbtn.c
>>> index 554e82ebe83c..1616cb9c4ae5 100644
>>> --- a/drivers/platform/x86/intel-vbtn.c
>>> +++ b/drivers/platform/x86/intel-vbtn.c
>>> @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
>>>  static const struct key_entry intel_vbtn_keymap[] = {
>>> { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
>>> { KE_KEY, 0xC1, { KEY_POWER } },/* power key release */
>>> +   { KE_KEY, 0xC4, { KEY_VOLUMEUP} },  /* volume-up key press */
>>> +   { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } },  /* volume-up key release */
>>> +   { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } },   /* volume-down key press */
>>> +   { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } },/* volume-down key 
>>> release */
>>> { KE_END },
>>>  };
>>>
>>> --
>>> 2.12.2
>>>
>>
>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>
>
>
> --
> Far away from the primal instinct, the song seems to fade away, the
> river get wider between your thoughts and the things we do and say.

Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

[AceLan Kao]

According the spec. I have, the values are correct.
Please merge it, thanks.

2017-04-25 5:41 GMT+08:00 Maarten Maathuis :
> On Mon, Apr 24, 2017 at 11:37 PM, Andy Shevchenko
>  wrote:
>> On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis  
>> wrote:
>>> Tested on HP Elite X2 1012 G1.
>>> Matches event report of Lenovo Helix 2
>>> (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).
>>>
>>
>> Much better!
>>
>>> V2: Fix indent and add sign-off
>>
>> Usually this line goes after --- (body delimiter).
>> No need to resend this time. I would wait a bit for actual
>> author/driver maintainer to comment. Otherwise patch looks good enough
>> to me.
>
> The intent is not have this in the commit message?
> I'll keep an eye out if i can place it below "---" next time.
> Although i suspect the message would end in the actual code diff,
> which seems odd.
>
>>
>>>
>>> Signed-off-by: Maarten Maathuis 
>>> ---
>>>  drivers/platform/x86/intel-vbtn.c | 4 
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/intel-vbtn.c 
>>> b/drivers/platform/x86/intel-vbtn.c
>>> index 554e82ebe83c..1616cb9c4ae5 100644
>>> --- a/drivers/platform/x86/intel-vbtn.c
>>> +++ b/drivers/platform/x86/intel-vbtn.c
>>> @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
>>>  static const struct key_entry intel_vbtn_keymap[] = {
>>> { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
>>> { KE_KEY, 0xC1, { KEY_POWER } },/* power key release */
>>> +   { KE_KEY, 0xC4, { KEY_VOLUMEUP} },  /* volume-up key press */
>>> +   { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } },  /* volume-up key release */
>>> +   { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } },   /* volume-down key press */
>>> +   { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } },/* volume-down key 
>>> release */
>>> { KE_END },
>>>  };
>>>
>>> --
>>> 2.12.2
>>>
>>
>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>
>
>
> --
> Far away from the primal instinct, the song seems to fade away, the
> river get wider between your thoughts and the things we do and say.

Re: [PATCH v3 2/2] Input: add support for the STMicroelectronics FingerTip touchscreen

[Andi Shyti]

Hi Dmitry,

again, kindly ping. This patch has been posted on March 27th and
this is the third time I ask you a feedback about it.

Please let me know,
Andi

On Mon, Mar 27, 2017 at 10:07:43PM +0900, Andi Shyti wrote:
> The stmfts (ST-Microelectronics FingerTip S) touchscreen device
> is a capacitive multi-touch controller mainly for mobile use.
> 
> It's connected through i2c bus at the address 0x49 and it
> interfaces with userspace through input event interface.
> 
> At the current state it provides a touchscreen multitouch
> functionality up to 10 fingers. Each finger is enumerated with a
> distinctive id (from 0 to 9).
> 
> If enabled the device can support single "touch" hovering, by
> providing three coordinates, x, y and distance.
> 
> It is possible to select the touchkey functionality which
> provides a basic two keys interface for "home" and "back" menu,
> typical in mobile phones.
> 
> Signed-off-by: Andi Shyti 
> ---
>  drivers/input/touchscreen/Kconfig  |  12 +
>  drivers/input/touchscreen/Makefile |   1 +
>  drivers/input/touchscreen/stmfts.c | 805 
> +
>  3 files changed, 818 insertions(+)
>  create mode 100644 drivers/input/touchscreen/stmfts.c
> 
> diff --git a/drivers/input/touchscreen/Kconfig 
> b/drivers/input/touchscreen/Kconfig
> index 33c62e5de4fa..f8631c64290d 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -1114,6 +1114,18 @@ config TOUCHSCREEN_ST1232
> To compile this driver as a module, choose M here: the
> module will be called st1232_ts.
>  
> +config TOUCHSCREEN_STMFTS
> + tristate "STMicroelectronics STMFTS touchscreen"
> + depends on I2C
> + depends on INPUT
> + depends on LEDS_CLASS
> + help
> +   Say Y here if you want support for STMicroelectronics
> +   STMFTS touchscreen.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called stmfts.
> +
>  config TOUCHSCREEN_STMPE
>   tristate "STMicroelectronics STMPE touchscreens"
>   depends on MFD_STMPE
> diff --git a/drivers/input/touchscreen/Makefile 
> b/drivers/input/touchscreen/Makefile
> index 18e476948e44..6badce87037b 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -67,6 +67,7 @@ obj-$(CONFIG_TOUCHSCREEN_S3C2410)   += s3c2410_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_SILEAD) += silead.o
>  obj-$(CONFIG_TOUCHSCREEN_SIS_I2C)+= sis_i2c.o
>  obj-$(CONFIG_TOUCHSCREEN_ST1232) += st1232.o
> +obj-$(CONFIG_TOUCHSCREEN_STMFTS) += stmfts.o
>  obj-$(CONFIG_TOUCHSCREEN_STMPE)  += stmpe-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_SUN4I)  += sun4i-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_SUR40)  += sur40.o
> diff --git a/drivers/input/touchscreen/stmfts.c 
> b/drivers/input/touchscreen/stmfts.c
> new file mode 100644
> index ..2e18b1456f42
> --- /dev/null
> +++ b/drivers/input/touchscreen/stmfts.c
> @@ -0,0 +1,805 @@
> +/*
> + * Copyright (c) 2017 Samsung Electronics Co., Ltd.
> + * Author: Andi Shyti 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * STMicroelectronics FTS Touchscreen device driver
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* I2C commands */
> +#define STMFTS_READ_INFO 0x80
> +#define STMFTS_READ_STATUS   0x84
> +#define STMFTS_READ_ONE_EVENT0x85
> +#define STMFTS_READ_ALL_EVENT0x86
> +#define STMFTS_LATEST_EVENT  0x87
> +#define STMFTS_SLEEP_IN  0x90
> +#define STMFTS_SLEEP_OUT 0x91
> +#define STMFTS_MS_MT_SENSE_OFF   0x92
> +#define STMFTS_MS_MT_SENSE_ON0x93
> +#define STMFTS_SS_HOVER_SENSE_OFF0x94
> +#define STMFTS_SS_HOVER_SENSE_ON 0x95
> +#define STMFTS_MS_KEY_SENSE_OFF  0x9a
> +#define STMFTS_MS_KEY_SENSE_ON   0x9b
> +#define STMFTS_SYSTEM_RESET  0xa0
> +#define STMFTS_CLEAR_EVENT_STACK 0xa1
> +#define STMFTS_FULL_FORCE_CALIBRATION0xa2
> +#define STMFTS_MS_CX_TUNING  0xa3
> +#define STMFTS_SS_CX_TUNING  0xa4
> +
> +/* events */
> +#define STMFTS_EV_NO_EVENT   0x00
> +#define STMFTS_EV_MULTI_TOUCH_DETECTED   0x02
> +#define STMFTS_EV_MULTI_TOUCH_ENTER  0x03
> +#define STMFTS_EV_MULTI_TOUCH_LEAVE  0x04
> +#define STMFTS_EV_MULTI_TOUCH_MOTION 0x05
> +#define STMFTS_EV_HOVER_ENTER0x07
> +#define STMFTS_EV_HOVER_LEAVE

Re: [PATCH v3 2/2] Input: add support for the STMicroelectronics FingerTip touchscreen

[Andi Shyti]

Hi Dmitry,

again, kindly ping. This patch has been posted on March 27th and
this is the third time I ask you a feedback about it.

Please let me know,
Andi

On Mon, Mar 27, 2017 at 10:07:43PM +0900, Andi Shyti wrote:
> The stmfts (ST-Microelectronics FingerTip S) touchscreen device
> is a capacitive multi-touch controller mainly for mobile use.
> 
> It's connected through i2c bus at the address 0x49 and it
> interfaces with userspace through input event interface.
> 
> At the current state it provides a touchscreen multitouch
> functionality up to 10 fingers. Each finger is enumerated with a
> distinctive id (from 0 to 9).
> 
> If enabled the device can support single "touch" hovering, by
> providing three coordinates, x, y and distance.
> 
> It is possible to select the touchkey functionality which
> provides a basic two keys interface for "home" and "back" menu,
> typical in mobile phones.
> 
> Signed-off-by: Andi Shyti 
> ---
>  drivers/input/touchscreen/Kconfig  |  12 +
>  drivers/input/touchscreen/Makefile |   1 +
>  drivers/input/touchscreen/stmfts.c | 805 
> +
>  3 files changed, 818 insertions(+)
>  create mode 100644 drivers/input/touchscreen/stmfts.c
> 
> diff --git a/drivers/input/touchscreen/Kconfig 
> b/drivers/input/touchscreen/Kconfig
> index 33c62e5de4fa..f8631c64290d 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -1114,6 +1114,18 @@ config TOUCHSCREEN_ST1232
> To compile this driver as a module, choose M here: the
> module will be called st1232_ts.
>  
> +config TOUCHSCREEN_STMFTS
> + tristate "STMicroelectronics STMFTS touchscreen"
> + depends on I2C
> + depends on INPUT
> + depends on LEDS_CLASS
> + help
> +   Say Y here if you want support for STMicroelectronics
> +   STMFTS touchscreen.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called stmfts.
> +
>  config TOUCHSCREEN_STMPE
>   tristate "STMicroelectronics STMPE touchscreens"
>   depends on MFD_STMPE
> diff --git a/drivers/input/touchscreen/Makefile 
> b/drivers/input/touchscreen/Makefile
> index 18e476948e44..6badce87037b 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -67,6 +67,7 @@ obj-$(CONFIG_TOUCHSCREEN_S3C2410)   += s3c2410_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_SILEAD) += silead.o
>  obj-$(CONFIG_TOUCHSCREEN_SIS_I2C)+= sis_i2c.o
>  obj-$(CONFIG_TOUCHSCREEN_ST1232) += st1232.o
> +obj-$(CONFIG_TOUCHSCREEN_STMFTS) += stmfts.o
>  obj-$(CONFIG_TOUCHSCREEN_STMPE)  += stmpe-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_SUN4I)  += sun4i-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_SUR40)  += sur40.o
> diff --git a/drivers/input/touchscreen/stmfts.c 
> b/drivers/input/touchscreen/stmfts.c
> new file mode 100644
> index ..2e18b1456f42
> --- /dev/null
> +++ b/drivers/input/touchscreen/stmfts.c
> @@ -0,0 +1,805 @@
> +/*
> + * Copyright (c) 2017 Samsung Electronics Co., Ltd.
> + * Author: Andi Shyti 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * STMicroelectronics FTS Touchscreen device driver
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* I2C commands */
> +#define STMFTS_READ_INFO 0x80
> +#define STMFTS_READ_STATUS   0x84
> +#define STMFTS_READ_ONE_EVENT0x85
> +#define STMFTS_READ_ALL_EVENT0x86
> +#define STMFTS_LATEST_EVENT  0x87
> +#define STMFTS_SLEEP_IN  0x90
> +#define STMFTS_SLEEP_OUT 0x91
> +#define STMFTS_MS_MT_SENSE_OFF   0x92
> +#define STMFTS_MS_MT_SENSE_ON0x93
> +#define STMFTS_SS_HOVER_SENSE_OFF0x94
> +#define STMFTS_SS_HOVER_SENSE_ON 0x95
> +#define STMFTS_MS_KEY_SENSE_OFF  0x9a
> +#define STMFTS_MS_KEY_SENSE_ON   0x9b
> +#define STMFTS_SYSTEM_RESET  0xa0
> +#define STMFTS_CLEAR_EVENT_STACK 0xa1
> +#define STMFTS_FULL_FORCE_CALIBRATION0xa2
> +#define STMFTS_MS_CX_TUNING  0xa3
> +#define STMFTS_SS_CX_TUNING  0xa4
> +
> +/* events */
> +#define STMFTS_EV_NO_EVENT   0x00
> +#define STMFTS_EV_MULTI_TOUCH_DETECTED   0x02
> +#define STMFTS_EV_MULTI_TOUCH_ENTER  0x03
> +#define STMFTS_EV_MULTI_TOUCH_LEAVE  0x04
> +#define STMFTS_EV_MULTI_TOUCH_MOTION 0x05
> +#define STMFTS_EV_HOVER_ENTER0x07
> +#define STMFTS_EV_HOVER_LEAVE0x08
> +#define STMFTS_EV_HOVER_MOTION 

Re: [PATCH v7 4/5] i2c: aspeed: added driver for Aspeed I2C

[Benjamin Herrenschmidt]

On Mon, 2017-04-24 at 11:18 -0700, Brendan Higgins wrote:
> +static int __aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
> +    struct platform_device *pdev)
> +{

Minor nit ... I'm really not fan of those underscores.

We use __ functions in some cases in the kernel for low level
helpers, usually when it's a low level variant of an existing
function or an "unlocked" variant, but I don't think generalizing
it to pretty much everything in the driver is worthwhile here.

If you want to be explicit about locking, I would suggest you
use a comment in front of the function explaining if it
expects to be called with the lock held.

We tend to only do that when *both* functions exist and one is
implemented in term of the other.

Cheers,
Ben.

Re: [PATCH v7 4/5] i2c: aspeed: added driver for Aspeed I2C

[Benjamin Herrenschmidt]

On Mon, 2017-04-24 at 11:18 -0700, Brendan Higgins wrote:
> +static int __aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
> +    struct platform_device *pdev)
> +{

Minor nit ... I'm really not fan of those underscores.

We use __ functions in some cases in the kernel for low level
helpers, usually when it's a low level variant of an existing
function or an "unlocked" variant, but I don't think generalizing
it to pretty much everything in the driver is worthwhile here.

If you want to be explicit about locking, I would suggest you
use a comment in front of the function explaining if it
expects to be called with the lock held.

We tend to only do that when *both* functions exist and one is
implemented in term of the other.

Cheers,
Ben.

Re: [PATCH v6 4/5] i2c: aspeed: added driver for Aspeed I2C

[Benjamin Herrenschmidt]

On Mon, 2017-04-24 at 11:56 -0700, Brendan Higgins wrote:
> > > +struct aspeed_i2c_bus {
> > > + struct i2c_adapter  adap;
> > > + struct device   *dev;
> > > + void __iomem*base;
> > > + /* Synchronizes I/O mem access to base. */
> > > + spinlock_t  lock;
> > 
> > I am not entirely convinced we need that lock. The i2c core will
> > take a mutex protecting all operations on the bus. So we only need
> > to synchronize between our "xfer" code and our interrupt handler.
> 
> You are right if both having slave and master active at the same time
> was not possible; however, it is.

Right, I somewhat forgot about the slave case.

  ...

> > Some of those error states probably also warrant a reset of the
> > controller,
> > I think aspeed does that in the SDK.
> 
> For timeout and cmd_err, I do not see any argument against it; it
> sounds like we are in a very messed up, very unknown state, so full
> reset is probably the best last resort.

Yup.

> For SDA staying pulled down, I
> think we can say with reasonable confidence that some device on our
> bus is behaving very badly and I am not convinced that resetting the
> controller is likely to do anything to help;

Right. Hammering with STOPs and pray ...

>  that being said, I really
> do not have any good ideas to address that. So maybe praying and
> resetting the controller is *the most reasonable thing to do.* I
> would like to know what you think we should do in that case.

Well, there's a (small ?) chance that it's a controller bug asserting
the line so ... but there's little we can do if not.

> While I was thinking about this I also realized that the SDA line
> check after recovery happens in the else branch, but SCL line check
> does not happen after we attempt to STOP if SCL is hung. If we decide
> to make special note SDA being hung by a device that won't let go, we
> might want to make a special note that SCL is hung by a device that
> won't let go. Just a thought.

Maybe. Or just "unrecoverable error"... hopefully these don't happen
too often ... We had cases of a TPM misbehaving like that.

> > > +out:
> 
> ...
> > What about I2C_M_NOSTART ?
> > 
> > Not that I've ever seen it used... ;-)
> 
> Right now I am not doing any of the protocol mangling options, but I
> can add them in if you think it is important for initial support.

No, not important, we can add that later if it ever becomes useful.

 ...

> > In general, you always ACK all interrupts first. Then you handle
> > the bits you have harvested.
> > 
> 
> The documentation says to ACK the interrupt after handling in the RX
> case:
> 
> <<<
> S/W needs to clear this status bit to allow next data receiving.
> > > > 
> 
> I will double check with Ryan to make sure TX works the same way.
> 
> > > + if (irq_status & ASPEED_I2CD_INTR_ERROR ||
> > > + (!bus->msgs && bus->master_state !=
> > > ASPEED_I2C_MASTER_STOP)) {
> 
> ...
> > 
> > I would set master_state to "RECOVERY" (new state ?) and ensure
> > those things are caught if they happen outside of a recovery.

I replied privately ... as long as we ack before we start a new command
we should be ok but we shouldn't ack after.

Your latest patch still does that. It will do things like start a STOP
command *then* ack the status bits. I'm pretty sure that's bogus.

That way it's a lot simpler to simply move the

writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);

To either right after the readl of the status reg at the beginning of
aspeed_i2c_master_irq().

I would be very surprised if that didn't work properly and wasn't much
safer than what you are currently doing. 

> Let me know if you still think we need a "RECOVERY" state.

The way you just switch to stop state and store the error for later
should work I think.

> > 
> > > + if (bus->master_state == ASPEED_I2C_MASTER_START) {
> 
> ...
> > 
> > > + dev_dbg(bus->dev,
> > > + "no slave present at %02x", msg-
> > > >addr);
> > > + status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> > > + bus->cmd_err = -EIO;
> > > + do_stop(bus);
> > > + goto out_no_complete;
> > > + } else {
> > > + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> > > + if (msg->flags & I2C_M_RD)
> > > + bus->master_state =
> > > ASPEED_I2C_MASTER_RX;
> > > + else
> > > + bus->master_state =
> > > ASPEED_I2C_MASTER_TX_FIRST;
> > 
> > What about the SMBUS_QUICK case ? (0-len transfer). Do we need
> > to handle this here ? A quick look at the TX_FIRST case makes
> > me think we are ok there but I'm not sure about the RX case.
> 
> I did not think that there is an SMBUS_QUICK RX. Could you point me
> to an example?

Not so much an RX, it's more like you are sending a 1-bit data in
the 

Re: [PATCH v6 4/5] i2c: aspeed: added driver for Aspeed I2C

[Benjamin Herrenschmidt]

On Mon, 2017-04-24 at 11:56 -0700, Brendan Higgins wrote:
> > > +struct aspeed_i2c_bus {
> > > + struct i2c_adapter  adap;
> > > + struct device   *dev;
> > > + void __iomem*base;
> > > + /* Synchronizes I/O mem access to base. */
> > > + spinlock_t  lock;
> > 
> > I am not entirely convinced we need that lock. The i2c core will
> > take a mutex protecting all operations on the bus. So we only need
> > to synchronize between our "xfer" code and our interrupt handler.
> 
> You are right if both having slave and master active at the same time
> was not possible; however, it is.

Right, I somewhat forgot about the slave case.

  ...

> > Some of those error states probably also warrant a reset of the
> > controller,
> > I think aspeed does that in the SDK.
> 
> For timeout and cmd_err, I do not see any argument against it; it
> sounds like we are in a very messed up, very unknown state, so full
> reset is probably the best last resort.

Yup.

> For SDA staying pulled down, I
> think we can say with reasonable confidence that some device on our
> bus is behaving very badly and I am not convinced that resetting the
> controller is likely to do anything to help;

Right. Hammering with STOPs and pray ...

>  that being said, I really
> do not have any good ideas to address that. So maybe praying and
> resetting the controller is *the most reasonable thing to do.* I
> would like to know what you think we should do in that case.

Well, there's a (small ?) chance that it's a controller bug asserting
the line so ... but there's little we can do if not.

> While I was thinking about this I also realized that the SDA line
> check after recovery happens in the else branch, but SCL line check
> does not happen after we attempt to STOP if SCL is hung. If we decide
> to make special note SDA being hung by a device that won't let go, we
> might want to make a special note that SCL is hung by a device that
> won't let go. Just a thought.

Maybe. Or just "unrecoverable error"... hopefully these don't happen
too often ... We had cases of a TPM misbehaving like that.

> > > +out:
> 
> ...
> > What about I2C_M_NOSTART ?
> > 
> > Not that I've ever seen it used... ;-)
> 
> Right now I am not doing any of the protocol mangling options, but I
> can add them in if you think it is important for initial support.

No, not important, we can add that later if it ever becomes useful.

 ...

> > In general, you always ACK all interrupts first. Then you handle
> > the bits you have harvested.
> > 
> 
> The documentation says to ACK the interrupt after handling in the RX
> case:
> 
> <<<
> S/W needs to clear this status bit to allow next data receiving.
> > > > 
> 
> I will double check with Ryan to make sure TX works the same way.
> 
> > > + if (irq_status & ASPEED_I2CD_INTR_ERROR ||
> > > + (!bus->msgs && bus->master_state !=
> > > ASPEED_I2C_MASTER_STOP)) {
> 
> ...
> > 
> > I would set master_state to "RECOVERY" (new state ?) and ensure
> > those things are caught if they happen outside of a recovery.

I replied privately ... as long as we ack before we start a new command
we should be ok but we shouldn't ack after.

Your latest patch still does that. It will do things like start a STOP
command *then* ack the status bits. I'm pretty sure that's bogus.

That way it's a lot simpler to simply move the

writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);

To either right after the readl of the status reg at the beginning of
aspeed_i2c_master_irq().

I would be very surprised if that didn't work properly and wasn't much
safer than what you are currently doing. 

> Let me know if you still think we need a "RECOVERY" state.

The way you just switch to stop state and store the error for later
should work I think.

> > 
> > > + if (bus->master_state == ASPEED_I2C_MASTER_START) {
> 
> ...
> > 
> > > + dev_dbg(bus->dev,
> > > + "no slave present at %02x", msg-
> > > >addr);
> > > + status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> > > + bus->cmd_err = -EIO;
> > > + do_stop(bus);
> > > + goto out_no_complete;
> > > + } else {
> > > + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> > > + if (msg->flags & I2C_M_RD)
> > > + bus->master_state =
> > > ASPEED_I2C_MASTER_RX;
> > > + else
> > > + bus->master_state =
> > > ASPEED_I2C_MASTER_TX_FIRST;
> > 
> > What about the SMBUS_QUICK case ? (0-len transfer). Do we need
> > to handle this here ? A quick look at the TX_FIRST case makes
> > me think we are ok there but I'm not sure about the RX case.
> 
> I did not think that there is an SMBUS_QUICK RX. Could you point me
> to an example?

Not so much an RX, it's more like you are sending a 1-bit data in
the 

Re: [linux-sunxi] [PATCH v4 07/10] mfd: axp20x: add axp20x-regulator cell for AXP803

[Chen-Yu Tsai]

On Tue, Apr 25, 2017 at 12:01 AM, Icenowy Zheng  wrote:
> As axp20x-regulator now supports AXP803, add a cell for it.
>
> Signed-off-by: Icenowy Zheng 
> Acked-by: Chen-Yu Tsai 
> ---
> Changes in v4:
> - Added a trailing comma for new cell, for easier further cell addition.
> Changes in v3:
> - Make the new cell one-liner.
>
>  drivers/mfd/axp20x.c | 3 ++-
>  drivers/regulator/axp20x-regulator.c | 6 +++---

Squashed in wrong patch?

ChenYu

Re: [linux-sunxi] [PATCH v4 07/10] mfd: axp20x: add axp20x-regulator cell for AXP803

[Chen-Yu Tsai]

On Tue, Apr 25, 2017 at 12:01 AM, Icenowy Zheng  wrote:
> As axp20x-regulator now supports AXP803, add a cell for it.
>
> Signed-off-by: Icenowy Zheng 
> Acked-by: Chen-Yu Tsai 
> ---
> Changes in v4:
> - Added a trailing comma for new cell, for easier further cell addition.
> Changes in v3:
> - Make the new cell one-liner.
>
>  drivers/mfd/axp20x.c | 3 ++-
>  drivers/regulator/axp20x-regulator.c | 6 +++---

Squashed in wrong patch?

ChenYu

[PATCH] irqchip/mbigen: Fix the clear register offset

[Majun]

From: MaJun 

Don't minus reserved interrupts (64) when get the clear register offset,because
the clear register space includes the space of these 64 interrupts.

Signed-off-by: MaJun 
---
 drivers/irqchip/irq-mbigen.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 061cdb8..75818a5 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -108,7 +108,6 @@ static inline void get_mbigen_clear_reg(irq_hw_number_t 
hwirq,
 {
unsigned int ofst;
 
-   hwirq -= RESERVED_IRQ_PER_MBIGEN_CHIP;
ofst = hwirq / 32 * 4;
 
*mask = 1 << (hwirq % 32);
-- 
1.7.12.4

[PATCH] irqchip/mbigen: Fix the clear register offset

[Majun]

From: MaJun 

Don't minus reserved interrupts (64) when get the clear register offset,because
the clear register space includes the space of these 64 interrupts.

Signed-off-by: MaJun 
---
 drivers/irqchip/irq-mbigen.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 061cdb8..75818a5 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -108,7 +108,6 @@ static inline void get_mbigen_clear_reg(irq_hw_number_t 
hwirq,
 {
unsigned int ofst;
 
-   hwirq -= RESERVED_IRQ_PER_MBIGEN_CHIP;
ofst = hwirq / 32 * 4;
 
*mask = 1 << (hwirq % 32);
-- 
1.7.12.4

Re: [PATCH] scsi/mac_esp: Replace bogus memory barrier with spinlock

[Michael Schmitz]

Martin,

looks good to me, so:

Reviewed-By: Michael Schmitz 


Am 25.04.2017 um 10:29 schrieb Martin K. Petersen:
> 
> Finn,
> 
>> Commit da244654c66e ("[SCSI] mac_esp: fix for quadras with two esp chips")
>> added mac_scsi_esp_intr() to handle the IRQ lines from a pair of on-board
>> ESP chips (a normal shared IRQ did not work).
>>
>> Proper mutual exclusion was missing from that patch. This patch fixes
>> race conditions between comparison and assignment of esp_chips[]
>> pointers.
> 
> Ondrej/Michael: Mind reviewing this change?
> 

Re: [PATCH] scsi/mac_esp: Replace bogus memory barrier with spinlock

[Michael Schmitz]

Martin,

looks good to me, so:

Reviewed-By: Michael Schmitz 


Am 25.04.2017 um 10:29 schrieb Martin K. Petersen:
> 
> Finn,
> 
>> Commit da244654c66e ("[SCSI] mac_esp: fix for quadras with two esp chips")
>> added mac_scsi_esp_intr() to handle the IRQ lines from a pair of on-board
>> ESP chips (a normal shared IRQ did not work).
>>
>> Proper mutual exclusion was missing from that patch. This patch fixes
>> race conditions between comparison and assignment of esp_chips[]
>> pointers.
> 
> Ondrej/Michael: Mind reviewing this change?
> 

Re: Re: [PATCH 1/2] PCI: mediatek: Add Mediatek PCIe host controller support

[Ryder Lee]

Hi,

On Mon, 2017-04-24 at 17:02 -0500, Bjorn Helgaas wrote:
> Hi Ryder,
> 
> Looks good, but I have a few questions below.
> 
> On Sun, Apr 23, 2017 at 04:19:02PM +0800, Ryder Lee wrote:
> > Add support for the Mediatek PCIe controller which can be found
> > on MT7623A/N, MT2701 and MT8521p platforms.
> > 
> > Signed-off-by: Ryder Lee 
> > ---
> >  drivers/pci/host/Kconfig |  11 +
> >  drivers/pci/host/Makefile|   1 +
> >  drivers/pci/host/pcie-mediatek.c | 611 
> > +++
> >  3 files changed, 623 insertions(+)
> >  create mode 100644 drivers/pci/host/pcie-mediatek.c
> > 
> > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > index f7c1d4d..cf13b5d 100644
> > --- a/drivers/pci/host/Kconfig
> > +++ b/drivers/pci/host/Kconfig
> > @@ -174,6 +174,17 @@ config PCIE_ROCKCHIP
> >   There is 1 internal PCIe port available to support GEN2 with
> >   4 slots.
> >  
> > +config PCIE_MEDIATEK
> > +   bool "Mediatek PCIe Controller for MT7623 SoCs families"
> > +   depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST
> > +   depends on OF
> > +   depends on PCI
> > +   select PCIEPORTBUS
> > +   help
> > + Say Y here if you want to enable PCIe controller support on MT7623 A/N
> > + series SoCs. There is a one root complex with 3 root ports available.
> > + Each port supports Gen2 lane x1.
> > +
> >  config VMD
> > depends on PCI_MSI && X86_64 && SRCU
> > tristate "Intel Volume Management Device Driver"
> > diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> > index 4d36866..265adff 100644
> > --- a/drivers/pci/host/Makefile
> > +++ b/drivers/pci/host/Makefile
> > @@ -17,6 +17,7 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
> >  obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
> >  obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
> >  obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
> > +obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o
> >  obj-$(CONFIG_VMD) += vmd.o
> >  
> >  # The following drivers are for devices that use the generic ACPI
> > diff --git a/drivers/pci/host/pcie-mediatek.c 
> > b/drivers/pci/host/pcie-mediatek.c
> > new file mode 100644
> > index 000..98e84d9
> > --- /dev/null
> > +++ b/drivers/pci/host/pcie-mediatek.c
> > @@ -0,0 +1,611 @@
> > +/*
> > + * PCIe host controller driver for Mediatek MT7623 SoCs families
> > + *
> > + * Copyright (c) 2017 MediaTek Inc.
> > + * Author: Ryder Lee 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * 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.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/* PCIe shared registers */
> > +#define PCIE_SYS_CFG   0x00
> > +#define PCIE_INT_ENABLE0x0c
> > +#define PCIE_CFG_ADDR  0x20
> > +#define PCIE_CFG_DATA  0x24
> > +
> > +/* PCIe per port registers */
> > +#define PCIE_BAR0_SETUP0x10
> > +#define PCIE_BAR1_SETUP0x14
> > +#define PCIE_BAR0_MEM_BASE 0x18
> > +#define PCIE_CLASS 0x34
> > +#define PCIE_LINK_STATUS   0x50
> > +
> > +#define PCIE_PORT_INT_EN(x)BIT(20 + (x))
> > +#define PCIE_PORT_PERST(x) BIT(1 + (x))
> > +#define PCIE_PORT_LINKUP   BIT(0)
> > +#define PCIE_BAR_MAP_MAX   GENMASK(31, 16)
> > +
> > +#define PCIE_BAR_ENABLEBIT(0)
> > +#define PCIE_REVISION_ID   BIT(0)
> > +#define PCIE_CLASS_CODE(0x60400 << 8)
> > +#define PCIE_CONF_REG(regn)(((regn) & GENMASK(7, 2)) | \
> > +   regn) >> 8) & GENMASK(3, 0)) << 24))
> > +#define PCIE_CONF_FUN(fun) (((fun) << 8) & GENMASK(10, 8))
> > +#define PCIE_CONF_DEV(dev) (((dev) << 11) & GENMASK(15, 11))
> > +#define PCIE_CONF_BUS(bus) (((bus) << 16) & GENMASK(23, 16))
> > +#define PCIE_CONF_ADDR(regn, fun, dev, bus) \
> > +   (PCIE_CONF_REG(regn) | PCIE_CONF_FUN(fun) | \
> > +PCIE_CONF_DEV(dev) | PCIE_CONF_BUS(bus))
> > +
> > +/* Mediatek specific configuration registers */
> > +#define PCIE_FTS_NUM   0x70c
> > +#define PCIE_FTS_NUM_MASK  GENMASK(15, 8)
> > +#define PCIE_FTS_NUM_L0(x) ((x) & 0xff << 8)
> > +
> > +#define PCIE_FC_CREDIT 0x73c
> > +#define PCIE_FC_CREDIT_MASK(GENMASK(31, 31) | GENMASK(28, 16))
> > +#define PCIE_FC_CREDIT_VAL(x)  ((x) << 16)
> > +
> > +/**
> > + * struct mtk_pcie_port - PCIe port information
> > + * @dev: pointer to root port device
> > + * @base: IO mapped 

Re: Re: [PATCH 1/2] PCI: mediatek: Add Mediatek PCIe host controller support

[Ryder Lee]

Hi,

On Mon, 2017-04-24 at 17:02 -0500, Bjorn Helgaas wrote:
> Hi Ryder,
> 
> Looks good, but I have a few questions below.
> 
> On Sun, Apr 23, 2017 at 04:19:02PM +0800, Ryder Lee wrote:
> > Add support for the Mediatek PCIe controller which can be found
> > on MT7623A/N, MT2701 and MT8521p platforms.
> > 
> > Signed-off-by: Ryder Lee 
> > ---
> >  drivers/pci/host/Kconfig |  11 +
> >  drivers/pci/host/Makefile|   1 +
> >  drivers/pci/host/pcie-mediatek.c | 611 
> > +++
> >  3 files changed, 623 insertions(+)
> >  create mode 100644 drivers/pci/host/pcie-mediatek.c
> > 
> > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > index f7c1d4d..cf13b5d 100644
> > --- a/drivers/pci/host/Kconfig
> > +++ b/drivers/pci/host/Kconfig
> > @@ -174,6 +174,17 @@ config PCIE_ROCKCHIP
> >   There is 1 internal PCIe port available to support GEN2 with
> >   4 slots.
> >  
> > +config PCIE_MEDIATEK
> > +   bool "Mediatek PCIe Controller for MT7623 SoCs families"
> > +   depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST
> > +   depends on OF
> > +   depends on PCI
> > +   select PCIEPORTBUS
> > +   help
> > + Say Y here if you want to enable PCIe controller support on MT7623 A/N
> > + series SoCs. There is a one root complex with 3 root ports available.
> > + Each port supports Gen2 lane x1.
> > +
> >  config VMD
> > depends on PCI_MSI && X86_64 && SRCU
> > tristate "Intel Volume Management Device Driver"
> > diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> > index 4d36866..265adff 100644
> > --- a/drivers/pci/host/Makefile
> > +++ b/drivers/pci/host/Makefile
> > @@ -17,6 +17,7 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
> >  obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
> >  obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
> >  obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
> > +obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o
> >  obj-$(CONFIG_VMD) += vmd.o
> >  
> >  # The following drivers are for devices that use the generic ACPI
> > diff --git a/drivers/pci/host/pcie-mediatek.c 
> > b/drivers/pci/host/pcie-mediatek.c
> > new file mode 100644
> > index 000..98e84d9
> > --- /dev/null
> > +++ b/drivers/pci/host/pcie-mediatek.c
> > @@ -0,0 +1,611 @@
> > +/*
> > + * PCIe host controller driver for Mediatek MT7623 SoCs families
> > + *
> > + * Copyright (c) 2017 MediaTek Inc.
> > + * Author: Ryder Lee 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * 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.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/* PCIe shared registers */
> > +#define PCIE_SYS_CFG   0x00
> > +#define PCIE_INT_ENABLE0x0c
> > +#define PCIE_CFG_ADDR  0x20
> > +#define PCIE_CFG_DATA  0x24
> > +
> > +/* PCIe per port registers */
> > +#define PCIE_BAR0_SETUP0x10
> > +#define PCIE_BAR1_SETUP0x14
> > +#define PCIE_BAR0_MEM_BASE 0x18
> > +#define PCIE_CLASS 0x34
> > +#define PCIE_LINK_STATUS   0x50
> > +
> > +#define PCIE_PORT_INT_EN(x)BIT(20 + (x))
> > +#define PCIE_PORT_PERST(x) BIT(1 + (x))
> > +#define PCIE_PORT_LINKUP   BIT(0)
> > +#define PCIE_BAR_MAP_MAX   GENMASK(31, 16)
> > +
> > +#define PCIE_BAR_ENABLEBIT(0)
> > +#define PCIE_REVISION_ID   BIT(0)
> > +#define PCIE_CLASS_CODE(0x60400 << 8)
> > +#define PCIE_CONF_REG(regn)(((regn) & GENMASK(7, 2)) | \
> > +   regn) >> 8) & GENMASK(3, 0)) << 24))
> > +#define PCIE_CONF_FUN(fun) (((fun) << 8) & GENMASK(10, 8))
> > +#define PCIE_CONF_DEV(dev) (((dev) << 11) & GENMASK(15, 11))
> > +#define PCIE_CONF_BUS(bus) (((bus) << 16) & GENMASK(23, 16))
> > +#define PCIE_CONF_ADDR(regn, fun, dev, bus) \
> > +   (PCIE_CONF_REG(regn) | PCIE_CONF_FUN(fun) | \
> > +PCIE_CONF_DEV(dev) | PCIE_CONF_BUS(bus))
> > +
> > +/* Mediatek specific configuration registers */
> > +#define PCIE_FTS_NUM   0x70c
> > +#define PCIE_FTS_NUM_MASK  GENMASK(15, 8)
> > +#define PCIE_FTS_NUM_L0(x) ((x) & 0xff << 8)
> > +
> > +#define PCIE_FC_CREDIT 0x73c
> > +#define PCIE_FC_CREDIT_MASK(GENMASK(31, 31) | GENMASK(28, 16))
> > +#define PCIE_FC_CREDIT_VAL(x)  ((x) << 16)
> > +
> > +/**
> > + * struct mtk_pcie_port - PCIe port information
> > + * @dev: pointer to root port device
> > + * @base: IO mapped register base
> > + * @list: port list
> > + * @pcie: 

Re: support autofocus / autogain in libv4l2

[Mauro Carvalho Chehab]

Em Tue, 25 Apr 2017 00:07:01 +0200
Pavel Machek  escreveu:

> Hi!
> 
> > Please don't add a new application under lib/. It is fine if you want
> > some testing application, if the ones there aren't enough, but please
> > place it under contrib/test/.
> > 
> > You should likely take a look at v4l2grab first, as it could have
> > almost everything you would need.  
> 
> I really need some kind of video output. v4l2grab is not useful
> there. v4l2gl might be, but I don't think I have enough dependencies.

Well, you could use some app to show the snaps that v4l2grab takes.

Yeah, compiling v4l2gl on N9 can indeed be complex. I suspect that it 
shouldn't hard to compile xawtv there (probably disabling some optional
features).

> Umm, and it looks like libv4l can not automatically convert from
> GRBG10.. and if it could, going through RGB24 would probably be too
> slow on this device :-(.

I suspect it shouldn't be hard to add support for GRBG10. It already
supports 8 and 16 bits Bayer formats, at lib/libv4lconvert/bayer.c
(to both RGB and YUV formats).

How it would preform is another question ;)

> > IMO, the above belongs to a separate processing module under
> > lib/libv4lconvert/processing/  
> 
> Is there an example using autogain/autowhitebalance from
> libv4lconvert?

Well, if you plug a USB camera without those controls, it should
automatically expose controls for it, as if the device had such
controls.

Thanks,
Mauro

Re: support autofocus / autogain in libv4l2

[Mauro Carvalho Chehab]

Em Tue, 25 Apr 2017 00:07:01 +0200
Pavel Machek  escreveu:

> Hi!
> 
> > Please don't add a new application under lib/. It is fine if you want
> > some testing application, if the ones there aren't enough, but please
> > place it under contrib/test/.
> > 
> > You should likely take a look at v4l2grab first, as it could have
> > almost everything you would need.  
> 
> I really need some kind of video output. v4l2grab is not useful
> there. v4l2gl might be, but I don't think I have enough dependencies.

Well, you could use some app to show the snaps that v4l2grab takes.

Yeah, compiling v4l2gl on N9 can indeed be complex. I suspect that it 
shouldn't hard to compile xawtv there (probably disabling some optional
features).

> Umm, and it looks like libv4l can not automatically convert from
> GRBG10.. and if it could, going through RGB24 would probably be too
> slow on this device :-(.

I suspect it shouldn't be hard to add support for GRBG10. It already
supports 8 and 16 bits Bayer formats, at lib/libv4lconvert/bayer.c
(to both RGB and YUV formats).

How it would preform is another question ;)

> > IMO, the above belongs to a separate processing module under
> > lib/libv4lconvert/processing/  
> 
> Is there an example using autogain/autowhitebalance from
> libv4lconvert?

Well, if you plug a USB camera without those controls, it should
automatically expose controls for it, as if the device had such
controls.

Thanks,
Mauro

[PATCH] arm64: dts: exynos: Remove the te-gpios property in the TM2 boards

[Hoegeun Kwon]

The decon uses HW-TRIGGER, so TE interrupt is not necessary.
Therefore, remove the te-gpios property in the TM2 dts.

Signed-off-by: Hoegeun Kwon 
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts 
b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index 3ff9527..23191eb 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -60,7 +60,6 @@
vci-supply = <_reg>;
reset-gpios = < 0 GPIO_ACTIVE_LOW>;
enable-gpios = < 5 GPIO_ACTIVE_HIGH>;
-   te-gpios = < 3 GPIO_ACTIVE_HIGH>;
};
 };
 
-- 
1.9.1

[PATCH] arm64: dts: exynos: Remove the te-gpios property in the TM2 boards

[Hoegeun Kwon]

The decon uses HW-TRIGGER, so TE interrupt is not necessary.
Therefore, remove the te-gpios property in the TM2 dts.

Signed-off-by: Hoegeun Kwon 
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts 
b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index 3ff9527..23191eb 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -60,7 +60,6 @@
vci-supply = <_reg>;
reset-gpios = < 0 GPIO_ACTIVE_LOW>;
enable-gpios = < 5 GPIO_ACTIVE_HIGH>;
-   te-gpios = < 3 GPIO_ACTIVE_HIGH>;
};
 };
 
-- 
1.9.1