Re: [PATCH] swiotlb: swiotlb_{alloc,free}_buffer should depend on CONFIG_DMA_DIRECT_OPS

[Konrad Rzeszutek Wilk]

On Sat, Mar 24, 2018 at 06:03:51PM +0100, Christoph Hellwig wrote:
> On Fri, Mar 23, 2018 at 02:57:07PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Fri, Mar 23, 2018 at 06:49:30PM +0100, Christoph Hellwig wrote:
> > > Otherwise we might get unused symbol warnings for configs that built
> > > swiotlb.c only for use by xen-swiotlb.c and that don't otherwise select
> > > CONFIG_DMA_DIRECT_OPS, which is possible on arm.
> > > 
> > > Fixes: 16e73adbca76 ("dma/swiotlb: Remove 
> > > swiotlb_{alloc,free}_coherent()")
> > > Reported-by: Stephen Rothwell 
> > > Signed-off-by: Christoph Hellwig 
> > 
> > 
> > Alternatively could we set the Kconfig to slect DMA_DIRECT_OPS?
> 
> 
> IFF we build swiotlb.c only for xen-swiotlb we don't need DMA_DIRECT_OPS.

I don't think there is ever an case where you want a Xen specific build.

> IF you are fine with just requring it as well that would be doable nicely
> only if we consolidate to a single defintion of CONFIG_SWIOTLB instead
> of one per architecture.  Which is something we should do anyway,
> so I'll look into it once I get some time.

Sounds good. Thank you.

Re: [PATCH] swiotlb: swiotlb_{alloc,free}_buffer should depend on CONFIG_DMA_DIRECT_OPS

[Konrad Rzeszutek Wilk]

On Sat, Mar 24, 2018 at 06:03:51PM +0100, Christoph Hellwig wrote:
> On Fri, Mar 23, 2018 at 02:57:07PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Fri, Mar 23, 2018 at 06:49:30PM +0100, Christoph Hellwig wrote:
> > > Otherwise we might get unused symbol warnings for configs that built
> > > swiotlb.c only for use by xen-swiotlb.c and that don't otherwise select
> > > CONFIG_DMA_DIRECT_OPS, which is possible on arm.
> > > 
> > > Fixes: 16e73adbca76 ("dma/swiotlb: Remove 
> > > swiotlb_{alloc,free}_coherent()")
> > > Reported-by: Stephen Rothwell 
> > > Signed-off-by: Christoph Hellwig 
> > 
> > 
> > Alternatively could we set the Kconfig to slect DMA_DIRECT_OPS?
> 
> 
> IFF we build swiotlb.c only for xen-swiotlb we don't need DMA_DIRECT_OPS.

I don't think there is ever an case where you want a Xen specific build.

> IF you are fine with just requring it as well that would be doable nicely
> only if we consolidate to a single defintion of CONFIG_SWIOTLB instead
> of one per architecture.  Which is something we should do anyway,
> so I'll look into it once I get some time.

Sounds good. Thank you.

Re: [PATCH v1 12/16] rtc: mediatek: cleanup header files to include

[kbuild test robot]

Hi Sean,

I love your patch! Yet something to improve:

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v4.16-rc6]
[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/sean-wang-mediatek-com/Add-support-to-MT6323-RTC-and-its-power-device/20180325-104529
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git 
rtc-next
config: tile-allmodconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=tile 

All errors (new ones prefixed by >>):

   drivers//rtc/rtc-mt6397.c: In function 'mtk_rtc_probe':
>> drivers//rtc/rtc-mt6397.c:270:13: error: implicit declaration of function 
>> 'irq_create_mapping'; did you mean 'page_mapping'? 
>> [-Werror=implicit-function-declaration]
 rtc->irq = irq_create_mapping(mt6397_chip->irq_domain, res->start);
^~
page_mapping
   cc1: some warnings being treated as errors

vim +270 drivers//rtc/rtc-mt6397.c

fc2979118 Tianping Fang  2015-05-06  254  
fc2979118 Tianping Fang  2015-05-06  255  static int mtk_rtc_probe(struct 
platform_device *pdev)
fc2979118 Tianping Fang  2015-05-06  256  {
fc2979118 Tianping Fang  2015-05-06  257struct resource *res;
fc2979118 Tianping Fang  2015-05-06  258struct mt6397_chip *mt6397_chip 
= dev_get_drvdata(pdev->dev.parent);
fc2979118 Tianping Fang  2015-05-06  259struct mt6397_rtc *rtc;
fc2979118 Tianping Fang  2015-05-06  260int ret;
fc2979118 Tianping Fang  2015-05-06  261  
fc2979118 Tianping Fang  2015-05-06  262rtc = devm_kzalloc(>dev, 
sizeof(struct mt6397_rtc), GFP_KERNEL);
fc2979118 Tianping Fang  2015-05-06  263if (!rtc)
fc2979118 Tianping Fang  2015-05-06  264return -ENOMEM;
fc2979118 Tianping Fang  2015-05-06  265  
fc2979118 Tianping Fang  2015-05-06  266res = 
platform_get_resource(pdev, IORESOURCE_MEM, 0);
fc2979118 Tianping Fang  2015-05-06  267rtc->addr_base = res->start;
fc2979118 Tianping Fang  2015-05-06  268  
fc2979118 Tianping Fang  2015-05-06  269res = 
platform_get_resource(pdev, IORESOURCE_IRQ, 0);
fc2979118 Tianping Fang  2015-05-06 @270rtc->irq = 
irq_create_mapping(mt6397_chip->irq_domain, res->start);
fc2979118 Tianping Fang  2015-05-06  271if (rtc->irq <= 0)
fc2979118 Tianping Fang  2015-05-06  272return -EINVAL;
fc2979118 Tianping Fang  2015-05-06  273  
fc2979118 Tianping Fang  2015-05-06  274rtc->regmap = 
mt6397_chip->regmap;
fc2979118 Tianping Fang  2015-05-06  275rtc->dev = >dev;
fc2979118 Tianping Fang  2015-05-06  276mutex_init(>lock);
fc2979118 Tianping Fang  2015-05-06  277  
fc2979118 Tianping Fang  2015-05-06  278platform_set_drvdata(pdev, rtc);
fc2979118 Tianping Fang  2015-05-06  279  
66c231b40 Sean Wang  2018-03-23  280ret = 
devm_request_threaded_irq(>dev, rtc->irq, NULL,
fc2979118 Tianping Fang  2015-05-06  281
mtk_rtc_irq_handler_thread,
fc2979118 Tianping Fang  2015-05-06  282
IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
fc2979118 Tianping Fang  2015-05-06  283
"mt6397-rtc", rtc);
fc2979118 Tianping Fang  2015-05-06  284if (ret) {
fc2979118 Tianping Fang  2015-05-06  285dev_err(>dev, 
"Failed to request alarm IRQ: %d: %d\n",
fc2979118 Tianping Fang  2015-05-06  286rtc->irq, ret);
63044753b Sean Wang  2018-03-23  287return ret;
fc2979118 Tianping Fang  2015-05-06  288}
fc2979118 Tianping Fang  2015-05-06  289  
baeca4495 Wei-Ning Huang 2015-07-02  290device_init_wakeup(>dev, 
1);
baeca4495 Wei-Ning Huang 2015-07-02  291  
66c231b40 Sean Wang  2018-03-23  292rtc->rtc_dev = 
devm_rtc_device_register(>dev, "mt6397-rtc",
fc2979118 Tianping Fang  2015-05-06  293
_rtc_ops, THIS_MODULE);
fc2979118 Tianping Fang  2015-05-06  294if (IS_ERR(rtc->rtc_dev)) {
fc2979118 Tianping Fang  2015-05-06  295dev_err(>dev, 
"register rtc device failed\n");
fc2979118 Tianping Fang  2015-05-06  296ret = 
PTR_ERR(rtc->rtc_dev);
fc2979118 Tianping Fang  2015-05-06  297return ret;
fc2979118 Tianping Fang  2015-05-06  298}
fc2979118 Tianping Fang  2015-05-06  299  
89a68f3c0 Sean Wang  2018-03-23  300return 
devm_of_platform_populate(>dev);
fc2979118 Tianping Fang  2015-05-06  301  }
fc2979118 Tianping Fang  2015-05-06  302  

:: The code at line 270 was first introduced by commit
:: 

Re: [PATCH v1 12/16] rtc: mediatek: cleanup header files to include

[kbuild test robot]

Hi Sean,

I love your patch! Yet something to improve:

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v4.16-rc6]
[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/sean-wang-mediatek-com/Add-support-to-MT6323-RTC-and-its-power-device/20180325-104529
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git 
rtc-next
config: tile-allmodconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=tile 

All errors (new ones prefixed by >>):

   drivers//rtc/rtc-mt6397.c: In function 'mtk_rtc_probe':
>> drivers//rtc/rtc-mt6397.c:270:13: error: implicit declaration of function 
>> 'irq_create_mapping'; did you mean 'page_mapping'? 
>> [-Werror=implicit-function-declaration]
 rtc->irq = irq_create_mapping(mt6397_chip->irq_domain, res->start);
^~
page_mapping
   cc1: some warnings being treated as errors

vim +270 drivers//rtc/rtc-mt6397.c

fc2979118 Tianping Fang  2015-05-06  254  
fc2979118 Tianping Fang  2015-05-06  255  static int mtk_rtc_probe(struct 
platform_device *pdev)
fc2979118 Tianping Fang  2015-05-06  256  {
fc2979118 Tianping Fang  2015-05-06  257struct resource *res;
fc2979118 Tianping Fang  2015-05-06  258struct mt6397_chip *mt6397_chip 
= dev_get_drvdata(pdev->dev.parent);
fc2979118 Tianping Fang  2015-05-06  259struct mt6397_rtc *rtc;
fc2979118 Tianping Fang  2015-05-06  260int ret;
fc2979118 Tianping Fang  2015-05-06  261  
fc2979118 Tianping Fang  2015-05-06  262rtc = devm_kzalloc(>dev, 
sizeof(struct mt6397_rtc), GFP_KERNEL);
fc2979118 Tianping Fang  2015-05-06  263if (!rtc)
fc2979118 Tianping Fang  2015-05-06  264return -ENOMEM;
fc2979118 Tianping Fang  2015-05-06  265  
fc2979118 Tianping Fang  2015-05-06  266res = 
platform_get_resource(pdev, IORESOURCE_MEM, 0);
fc2979118 Tianping Fang  2015-05-06  267rtc->addr_base = res->start;
fc2979118 Tianping Fang  2015-05-06  268  
fc2979118 Tianping Fang  2015-05-06  269res = 
platform_get_resource(pdev, IORESOURCE_IRQ, 0);
fc2979118 Tianping Fang  2015-05-06 @270rtc->irq = 
irq_create_mapping(mt6397_chip->irq_domain, res->start);
fc2979118 Tianping Fang  2015-05-06  271if (rtc->irq <= 0)
fc2979118 Tianping Fang  2015-05-06  272return -EINVAL;
fc2979118 Tianping Fang  2015-05-06  273  
fc2979118 Tianping Fang  2015-05-06  274rtc->regmap = 
mt6397_chip->regmap;
fc2979118 Tianping Fang  2015-05-06  275rtc->dev = >dev;
fc2979118 Tianping Fang  2015-05-06  276mutex_init(>lock);
fc2979118 Tianping Fang  2015-05-06  277  
fc2979118 Tianping Fang  2015-05-06  278platform_set_drvdata(pdev, rtc);
fc2979118 Tianping Fang  2015-05-06  279  
66c231b40 Sean Wang  2018-03-23  280ret = 
devm_request_threaded_irq(>dev, rtc->irq, NULL,
fc2979118 Tianping Fang  2015-05-06  281
mtk_rtc_irq_handler_thread,
fc2979118 Tianping Fang  2015-05-06  282
IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
fc2979118 Tianping Fang  2015-05-06  283
"mt6397-rtc", rtc);
fc2979118 Tianping Fang  2015-05-06  284if (ret) {
fc2979118 Tianping Fang  2015-05-06  285dev_err(>dev, 
"Failed to request alarm IRQ: %d: %d\n",
fc2979118 Tianping Fang  2015-05-06  286rtc->irq, ret);
63044753b Sean Wang  2018-03-23  287return ret;
fc2979118 Tianping Fang  2015-05-06  288}
fc2979118 Tianping Fang  2015-05-06  289  
baeca4495 Wei-Ning Huang 2015-07-02  290device_init_wakeup(>dev, 
1);
baeca4495 Wei-Ning Huang 2015-07-02  291  
66c231b40 Sean Wang  2018-03-23  292rtc->rtc_dev = 
devm_rtc_device_register(>dev, "mt6397-rtc",
fc2979118 Tianping Fang  2015-05-06  293
_rtc_ops, THIS_MODULE);
fc2979118 Tianping Fang  2015-05-06  294if (IS_ERR(rtc->rtc_dev)) {
fc2979118 Tianping Fang  2015-05-06  295dev_err(>dev, 
"register rtc device failed\n");
fc2979118 Tianping Fang  2015-05-06  296ret = 
PTR_ERR(rtc->rtc_dev);
fc2979118 Tianping Fang  2015-05-06  297return ret;
fc2979118 Tianping Fang  2015-05-06  298}
fc2979118 Tianping Fang  2015-05-06  299  
89a68f3c0 Sean Wang  2018-03-23  300return 
devm_of_platform_populate(>dev);
fc2979118 Tianping Fang  2015-05-06  301  }
fc2979118 Tianping Fang  2015-05-06  302  

:: The code at line 270 was first introduced by commit
:: 

[PATCH 2/2] KVM: X86: Fix disable pv tlb flush when steal time is enabled

[Wanpeng Li]

From: Wanpeng Li 

PV TLB FLUSH can be turned on when steal time is enabled. The condition 
reverse when the patch is sent out for several rounds review by mistake. 
This patch fixes it.

Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Signed-off-by: Wanpeng Li 
---
 arch/x86/kernel/kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 31ac585..4f859cc 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -555,7 +555,7 @@ static void __init kvm_guest_init(void)
 
if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH) &&
!kvm_para_has_hint(KVM_HINTS_DEDICATED) &&
-   !kvm_para_has_feature(KVM_FEATURE_STEAL_TIME))
+   kvm_para_has_feature(KVM_FEATURE_STEAL_TIME))
pv_mmu_ops.flush_tlb_others = kvm_flush_tlb_others;
 
if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
@@ -651,7 +651,7 @@ static __init int kvm_setup_pv_tlb_flush(void)
 
if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH) &&
!kvm_para_has_hint(KVM_HINTS_DEDICATED) &&
-   !kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
+   kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
for_each_possible_cpu(cpu) {
zalloc_cpumask_var_node(per_cpu_ptr(&__pv_tlb_mask, 
cpu),
GFP_KERNEL, cpu_to_node(cpu));
-- 
2.7.4

[PATCH 2/2] KVM: X86: Fix disable pv tlb flush when steal time is enabled

[Wanpeng Li]

From: Wanpeng Li 

PV TLB FLUSH can be turned on when steal time is enabled. The condition 
reverse when the patch is sent out for several rounds review by mistake. 
This patch fixes it.

Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Signed-off-by: Wanpeng Li 
---
 arch/x86/kernel/kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 31ac585..4f859cc 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -555,7 +555,7 @@ static void __init kvm_guest_init(void)
 
if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH) &&
!kvm_para_has_hint(KVM_HINTS_DEDICATED) &&
-   !kvm_para_has_feature(KVM_FEATURE_STEAL_TIME))
+   kvm_para_has_feature(KVM_FEATURE_STEAL_TIME))
pv_mmu_ops.flush_tlb_others = kvm_flush_tlb_others;
 
if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
@@ -651,7 +651,7 @@ static __init int kvm_setup_pv_tlb_flush(void)
 
if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH) &&
!kvm_para_has_hint(KVM_HINTS_DEDICATED) &&
-   !kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
+   kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
for_each_possible_cpu(cpu) {
zalloc_cpumask_var_node(per_cpu_ptr(&__pv_tlb_mask, 
cpu),
GFP_KERNEL, cpu_to_node(cpu));
-- 
2.7.4

Re: [PATCH v1 12/16] rtc: mediatek: cleanup header files to include

[kbuild test robot]

Hi Sean,

I love your patch! Yet something to improve:

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v4.16-rc6 next-20180323]
[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/sean-wang-mediatek-com/Add-support-to-MT6323-RTC-and-its-power-device/20180325-104529
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git 
rtc-next
config: x86_64-randconfig-x004-201812 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers//rtc/rtc-mt6397.c: In function 'mtk_rtc_probe':
>> drivers//rtc/rtc-mt6397.c:270:13: error: implicit declaration of function 
>> 'irq_create_mapping'; did you mean 'qid_has_mapping'? 
>> [-Werror=implicit-function-declaration]
 rtc->irq = irq_create_mapping(mt6397_chip->irq_domain, res->start);
^~
qid_has_mapping
   Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR
   Cyclomatic Complexity 1 include/linux/err.h:IS_ERR
   Cyclomatic Complexity 1 include/linux/math64.h:div_s64_rem
   Cyclomatic Complexity 1 include/linux/math64.h:div_s64
   Cyclomatic Complexity 3 include/linux/ktime.h:ktime_compare
   Cyclomatic Complexity 1 include/linux/ktime.h:ktime_add_us
   Cyclomatic Complexity 1 include/linux/device.h:devm_kzalloc
   Cyclomatic Complexity 1 include/linux/pm_wakeup.h:device_set_wakeup_capable
   Cyclomatic Complexity 1 include/linux/pm_wakeup.h:device_set_wakeup_enable
   Cyclomatic Complexity 1 include/linux/pm_wakeup.h:device_init_wakeup
   Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata
   Cyclomatic Complexity 1 include/linux/device.h:dev_set_drvdata
   Cyclomatic Complexity 1 include/linux/platform_device.h:platform_set_drvdata
   Cyclomatic Complexity 1 include/linux/of_platform.h:devm_of_platform_populate
   Cyclomatic Complexity 1 drivers//rtc/rtc-mt6397.c:mtk_rtc_driver_init
   Cyclomatic Complexity 5 drivers//rtc/rtc-mt6397.c:mtk_rtc_probe
   Cyclomatic Complexity 11 drivers//rtc/rtc-mt6397.c:mtk_rtc_write_trigger
   Cyclomatic Complexity 6 drivers//rtc/rtc-mt6397.c:mtk_rtc_set_alarm
   Cyclomatic Complexity 2 drivers//rtc/rtc-mt6397.c:mtk_rtc_set_time
   Cyclomatic Complexity 4 drivers//rtc/rtc-mt6397.c:mtk_rtc_read_alarm
   Cyclomatic Complexity 2 drivers//rtc/rtc-mt6397.c:__mtk_rtc_read_time
   Cyclomatic Complexity 3 drivers//rtc/rtc-mt6397.c:mtk_rtc_read_time
   Cyclomatic Complexity 4 drivers//rtc/rtc-mt6397.c:mtk_rtc_irq_handler_thread
   Cyclomatic Complexity 1 drivers//rtc/rtc-mt6397.c:mtk_rtc_driver_exit
   cc1: some warnings being treated as errors

vim +270 drivers//rtc/rtc-mt6397.c

fc2979118 Tianping Fang  2015-05-06  254  
fc2979118 Tianping Fang  2015-05-06  255  static int mtk_rtc_probe(struct 
platform_device *pdev)
fc2979118 Tianping Fang  2015-05-06  256  {
fc2979118 Tianping Fang  2015-05-06  257struct resource *res;
fc2979118 Tianping Fang  2015-05-06  258struct mt6397_chip *mt6397_chip 
= dev_get_drvdata(pdev->dev.parent);
fc2979118 Tianping Fang  2015-05-06  259struct mt6397_rtc *rtc;
fc2979118 Tianping Fang  2015-05-06  260int ret;
fc2979118 Tianping Fang  2015-05-06  261  
fc2979118 Tianping Fang  2015-05-06  262rtc = devm_kzalloc(>dev, 
sizeof(struct mt6397_rtc), GFP_KERNEL);
fc2979118 Tianping Fang  2015-05-06  263if (!rtc)
fc2979118 Tianping Fang  2015-05-06  264return -ENOMEM;
fc2979118 Tianping Fang  2015-05-06  265  
fc2979118 Tianping Fang  2015-05-06  266res = 
platform_get_resource(pdev, IORESOURCE_MEM, 0);
fc2979118 Tianping Fang  2015-05-06  267rtc->addr_base = res->start;
fc2979118 Tianping Fang  2015-05-06  268  
fc2979118 Tianping Fang  2015-05-06  269res = 
platform_get_resource(pdev, IORESOURCE_IRQ, 0);
fc2979118 Tianping Fang  2015-05-06 @270rtc->irq = 
irq_create_mapping(mt6397_chip->irq_domain, res->start);
fc2979118 Tianping Fang  2015-05-06  271if (rtc->irq <= 0)
fc2979118 Tianping Fang  2015-05-06  272return -EINVAL;
fc2979118 Tianping Fang  2015-05-06  273  
fc2979118 Tianping Fang  2015-05-06  274rtc->regmap = 
mt6397_chip->regmap;
fc2979118 Tianping Fang  2015-05-06  275rtc->dev = >dev;
fc2979118 Tianping Fang  2015-05-06  276mutex_init(>lock);
fc2979118 Tianping Fang  2015-05-06  277  
fc2979118 Tianping Fang  2015-05-06  278platform_set_drvdata(pdev, rtc);
fc2979118 Tianping Fang  2015-05-06  279  
66c231b40 Sean Wang  2018-03-23  280ret = 
devm_request_threaded_irq(>dev, rtc->irq, NULL,
fc2979118 Tianping Fang  2015-05-06  281
mtk_rtc_irq_handler_thread,
fc2979118 Tianping Fang  2015-05-06  282
IRQF_ONESHOT | IRQF_TRIGGER_HIGH,

[PATCH 1/2] KVM: X86: Fix setup the virt_spin_lock_key before static key get initialized

[Wanpeng Li]

From: Wanpeng Li 

 static_key_disable_cpuslocked(): static key 'virt_spin_lock_key+0x0/0x20' used 
before call to jump_label_init()
 WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:161 
static_key_disable_cpuslocked+0x61/0x80
 RIP: 0010:static_key_disable_cpuslocked+0x61/0x80
 Call Trace:
  static_key_disable+0x16/0x20
  start_kernel+0x192/0x4b3
  secondary_startup_64+0xa5/0xb0

Qspinlock will be choosed when dedicated pCPUs are available, however, the 
static virt_spin_lock_key is set in kvm_spinlock_init() before 
jump_label_init() 
has been called, which will result in a WARN(). This patch fixes it by delaying
the virt_spin_lock_key setup to .smp_prepare_cpus().

Reported-by: Davidlohr Bueso 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Davidlohr Bueso 
Signed-off-by: Wanpeng Li 
---
Note: Peterz pointed out in the IRC we have to audit all the architectures that 
implement smp_prepare_boot_cpu() to see what they depend on if we want to move 
jump_label_init() before smp_prepare_boot_cpu(). So what this patch does is 
similar to the issue which handled in xen ca5d376e.

 arch/x86/kernel/kvm.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 4ccbff6..747c61b 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -454,6 +454,13 @@ static void __init sev_map_percpu_data(void)
 }
 
 #ifdef CONFIG_SMP
+static void __init kvm_smp_prepare_cpus(unsigned int max_cpus)
+{
+   native_smp_prepare_cpus(max_cpus);
+   if (kvm_para_has_hint(KVM_HINTS_DEDICATED))
+   static_branch_disable(_spin_lock_key);
+}
+
 static void __init kvm_smp_prepare_boot_cpu(void)
 {
/*
@@ -557,6 +564,7 @@ static void __init kvm_guest_init(void)
kvm_setup_vsyscall_timeinfo();
 
 #ifdef CONFIG_SMP
+   smp_ops.smp_prepare_cpus = kvm_smp_prepare_cpus;
smp_ops.smp_prepare_boot_cpu = kvm_smp_prepare_boot_cpu;
if (cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "x86/kvm:online",
  kvm_cpu_online, kvm_cpu_down_prepare) < 0)
@@ -737,10 +745,8 @@ void __init kvm_spinlock_init(void)
if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
return;
 
-   if (kvm_para_has_hint(KVM_HINTS_DEDICATED)) {
-   static_branch_disable(_spin_lock_key);
+   if (kvm_para_has_hint(KVM_HINTS_DEDICATED))
return;
-   }
 
__pv_init_lock_hash();
pv_lock_ops.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
-- 
2.7.4

Re: [PATCH v1 12/16] rtc: mediatek: cleanup header files to include

[kbuild test robot]

Hi Sean,

I love your patch! Yet something to improve:

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v4.16-rc6 next-20180323]
[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/sean-wang-mediatek-com/Add-support-to-MT6323-RTC-and-its-power-device/20180325-104529
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git 
rtc-next
config: x86_64-randconfig-x004-201812 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers//rtc/rtc-mt6397.c: In function 'mtk_rtc_probe':
>> drivers//rtc/rtc-mt6397.c:270:13: error: implicit declaration of function 
>> 'irq_create_mapping'; did you mean 'qid_has_mapping'? 
>> [-Werror=implicit-function-declaration]
 rtc->irq = irq_create_mapping(mt6397_chip->irq_domain, res->start);
^~
qid_has_mapping
   Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR
   Cyclomatic Complexity 1 include/linux/err.h:IS_ERR
   Cyclomatic Complexity 1 include/linux/math64.h:div_s64_rem
   Cyclomatic Complexity 1 include/linux/math64.h:div_s64
   Cyclomatic Complexity 3 include/linux/ktime.h:ktime_compare
   Cyclomatic Complexity 1 include/linux/ktime.h:ktime_add_us
   Cyclomatic Complexity 1 include/linux/device.h:devm_kzalloc
   Cyclomatic Complexity 1 include/linux/pm_wakeup.h:device_set_wakeup_capable
   Cyclomatic Complexity 1 include/linux/pm_wakeup.h:device_set_wakeup_enable
   Cyclomatic Complexity 1 include/linux/pm_wakeup.h:device_init_wakeup
   Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata
   Cyclomatic Complexity 1 include/linux/device.h:dev_set_drvdata
   Cyclomatic Complexity 1 include/linux/platform_device.h:platform_set_drvdata
   Cyclomatic Complexity 1 include/linux/of_platform.h:devm_of_platform_populate
   Cyclomatic Complexity 1 drivers//rtc/rtc-mt6397.c:mtk_rtc_driver_init
   Cyclomatic Complexity 5 drivers//rtc/rtc-mt6397.c:mtk_rtc_probe
   Cyclomatic Complexity 11 drivers//rtc/rtc-mt6397.c:mtk_rtc_write_trigger
   Cyclomatic Complexity 6 drivers//rtc/rtc-mt6397.c:mtk_rtc_set_alarm
   Cyclomatic Complexity 2 drivers//rtc/rtc-mt6397.c:mtk_rtc_set_time
   Cyclomatic Complexity 4 drivers//rtc/rtc-mt6397.c:mtk_rtc_read_alarm
   Cyclomatic Complexity 2 drivers//rtc/rtc-mt6397.c:__mtk_rtc_read_time
   Cyclomatic Complexity 3 drivers//rtc/rtc-mt6397.c:mtk_rtc_read_time
   Cyclomatic Complexity 4 drivers//rtc/rtc-mt6397.c:mtk_rtc_irq_handler_thread
   Cyclomatic Complexity 1 drivers//rtc/rtc-mt6397.c:mtk_rtc_driver_exit
   cc1: some warnings being treated as errors

vim +270 drivers//rtc/rtc-mt6397.c

fc2979118 Tianping Fang  2015-05-06  254  
fc2979118 Tianping Fang  2015-05-06  255  static int mtk_rtc_probe(struct 
platform_device *pdev)
fc2979118 Tianping Fang  2015-05-06  256  {
fc2979118 Tianping Fang  2015-05-06  257struct resource *res;
fc2979118 Tianping Fang  2015-05-06  258struct mt6397_chip *mt6397_chip 
= dev_get_drvdata(pdev->dev.parent);
fc2979118 Tianping Fang  2015-05-06  259struct mt6397_rtc *rtc;
fc2979118 Tianping Fang  2015-05-06  260int ret;
fc2979118 Tianping Fang  2015-05-06  261  
fc2979118 Tianping Fang  2015-05-06  262rtc = devm_kzalloc(>dev, 
sizeof(struct mt6397_rtc), GFP_KERNEL);
fc2979118 Tianping Fang  2015-05-06  263if (!rtc)
fc2979118 Tianping Fang  2015-05-06  264return -ENOMEM;
fc2979118 Tianping Fang  2015-05-06  265  
fc2979118 Tianping Fang  2015-05-06  266res = 
platform_get_resource(pdev, IORESOURCE_MEM, 0);
fc2979118 Tianping Fang  2015-05-06  267rtc->addr_base = res->start;
fc2979118 Tianping Fang  2015-05-06  268  
fc2979118 Tianping Fang  2015-05-06  269res = 
platform_get_resource(pdev, IORESOURCE_IRQ, 0);
fc2979118 Tianping Fang  2015-05-06 @270rtc->irq = 
irq_create_mapping(mt6397_chip->irq_domain, res->start);
fc2979118 Tianping Fang  2015-05-06  271if (rtc->irq <= 0)
fc2979118 Tianping Fang  2015-05-06  272return -EINVAL;
fc2979118 Tianping Fang  2015-05-06  273  
fc2979118 Tianping Fang  2015-05-06  274rtc->regmap = 
mt6397_chip->regmap;
fc2979118 Tianping Fang  2015-05-06  275rtc->dev = >dev;
fc2979118 Tianping Fang  2015-05-06  276mutex_init(>lock);
fc2979118 Tianping Fang  2015-05-06  277  
fc2979118 Tianping Fang  2015-05-06  278platform_set_drvdata(pdev, rtc);
fc2979118 Tianping Fang  2015-05-06  279  
66c231b40 Sean Wang  2018-03-23  280ret = 
devm_request_threaded_irq(>dev, rtc->irq, NULL,
fc2979118 Tianping Fang  2015-05-06  281
mtk_rtc_irq_handler_thread,
fc2979118 Tianping Fang  2015-05-06  282
IRQF_ONESHOT | IRQF_TRIGGER_HIGH,

[PATCH 1/2] KVM: X86: Fix setup the virt_spin_lock_key before static key get initialized

[Wanpeng Li]

From: Wanpeng Li 

 static_key_disable_cpuslocked(): static key 'virt_spin_lock_key+0x0/0x20' used 
before call to jump_label_init()
 WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:161 
static_key_disable_cpuslocked+0x61/0x80
 RIP: 0010:static_key_disable_cpuslocked+0x61/0x80
 Call Trace:
  static_key_disable+0x16/0x20
  start_kernel+0x192/0x4b3
  secondary_startup_64+0xa5/0xb0

Qspinlock will be choosed when dedicated pCPUs are available, however, the 
static virt_spin_lock_key is set in kvm_spinlock_init() before 
jump_label_init() 
has been called, which will result in a WARN(). This patch fixes it by delaying
the virt_spin_lock_key setup to .smp_prepare_cpus().

Reported-by: Davidlohr Bueso 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Davidlohr Bueso 
Signed-off-by: Wanpeng Li 
---
Note: Peterz pointed out in the IRC we have to audit all the architectures that 
implement smp_prepare_boot_cpu() to see what they depend on if we want to move 
jump_label_init() before smp_prepare_boot_cpu(). So what this patch does is 
similar to the issue which handled in xen ca5d376e.

 arch/x86/kernel/kvm.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 4ccbff6..747c61b 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -454,6 +454,13 @@ static void __init sev_map_percpu_data(void)
 }
 
 #ifdef CONFIG_SMP
+static void __init kvm_smp_prepare_cpus(unsigned int max_cpus)
+{
+   native_smp_prepare_cpus(max_cpus);
+   if (kvm_para_has_hint(KVM_HINTS_DEDICATED))
+   static_branch_disable(_spin_lock_key);
+}
+
 static void __init kvm_smp_prepare_boot_cpu(void)
 {
/*
@@ -557,6 +564,7 @@ static void __init kvm_guest_init(void)
kvm_setup_vsyscall_timeinfo();
 
 #ifdef CONFIG_SMP
+   smp_ops.smp_prepare_cpus = kvm_smp_prepare_cpus;
smp_ops.smp_prepare_boot_cpu = kvm_smp_prepare_boot_cpu;
if (cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "x86/kvm:online",
  kvm_cpu_online, kvm_cpu_down_prepare) < 0)
@@ -737,10 +745,8 @@ void __init kvm_spinlock_init(void)
if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
return;
 
-   if (kvm_para_has_hint(KVM_HINTS_DEDICATED)) {
-   static_branch_disable(_spin_lock_key);
+   if (kvm_para_has_hint(KVM_HINTS_DEDICATED))
return;
-   }
 
__pv_init_lock_hash();
pv_lock_ops.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
-- 
2.7.4

Re: [PATCH v1 11/16] rtc: mediatek: move the declaration into a globally visible header file

[Sean Wang]

On Sat, 2018-03-24 at 17:00 -0300, Fabio Estevam wrote:
> On Fri, Mar 23, 2018 at 6:15 AM,   wrote:
> 
> > --- /dev/null
> > +++ b/include/linux/rtc/mt6397.h
> > @@ -0,0 +1,72 @@
> > +
> > +// SPDX-License-Identifier: GPL-2.0
> 
> According to Documentation/process/license-rules.rst the SPDX notation
> for C header file should be:
> 
> /* SPDX-License-Identifier: GPL-2.0 */


Thank you, I'll have an improvement according to the license rule.

Re: [PATCH v1 11/16] rtc: mediatek: move the declaration into a globally visible header file

[Sean Wang]

On Sat, 2018-03-24 at 17:00 -0300, Fabio Estevam wrote:
> On Fri, Mar 23, 2018 at 6:15 AM,   wrote:
> 
> > --- /dev/null
> > +++ b/include/linux/rtc/mt6397.h
> > @@ -0,0 +1,72 @@
> > +
> > +// SPDX-License-Identifier: GPL-2.0
> 
> According to Documentation/process/license-rules.rst the SPDX notation
> for C header file should be:
> 
> /* SPDX-License-Identifier: GPL-2.0 */


Thank you, I'll have an improvement according to the license rule.

Re: [v6] usb: ohci: Proper handling of ed_rm_list to handle race condition between usb_kill_urb() and finish_unlinks()

[Jonathan Liu]

Hi,

On 25 March 2018 at 12:21, Jonathan Liu  wrote:
> Hi,
>
> On 8 February 2018 at 14:55, Jeffy Chen  wrote:
>> From: AMAN DEEP 
>>
>> There is a race condition between finish_unlinks->finish_urb() function
>> and usb_kill_urb() in ohci controller case. The finish_urb calls
>> spin_unlock(>lock) before usb_hcd_giveback_urb() function call,
>> then if during this time, usb_kill_urb is called for another endpoint,
>> then new ed will be added to ed_rm_list at beginning for unlink, and
>> ed_rm_list will point to newly added.
>>
>> When finish_urb() is completed in finish_unlinks() and ed->td_list
>> becomes empty as in below code (in finish_unlinks() function):
>>
>> if (list_empty(>td_list)) {
>> *last = ed->ed_next;
>> ed->ed_next = NULL;
>> } else if (ohci->rh_state == OHCI_RH_RUNNING) {
>> *last = ed->ed_next;
>> ed->ed_next = NULL;
>> ed_schedule(ohci, ed);
>> }
>>
>> The *last = ed->ed_next will make ed_rm_list to point to ed->ed_next
>> and previously added ed by usb_kill_urb will be left unreferenced by
>> ed_rm_list. This causes usb_kill_urb() hang forever waiting for
>> finish_unlink to remove added ed from ed_rm_list.
>>
>> The main reason for hang in this race condtion is addition and removal
>> of ed from ed_rm_list in the beginning during usb_kill_urb and later
>> last* is modified in finish_unlinks().
>>
>> As suggested by Alan Stern, the solution for proper handling of
>> ohci->ed_rm_list is to remove ed from the ed_rm_list before finishing
>> any URBs. Then at the end, we can add ed back to the list if necessary.
>>
>> This properly handle the updated ohci->ed_rm_list in usb_kill_urb().
>>
>> Fixes:977dcfdc6031("USB:OHCI:don't lose track of EDs when a controller dies")
>> Acked-by: Alan Stern 
>> CC: 
>> Signed-off-by: Aman Deep 
>> Signed-off-by: Jeffy Chen 
>> ---
>>
>> Changes in v6:
>> This is a resend of Aman Deep's v5 patch [0], which solved the hang we
>> hit [1]. (Thanks Aman :)
>>
>> The v5 has some format issues, so i slightly adjust the commit message.
>>
>> [0] https://www.spinics.net/lists/linux-usb/msg129010.html
>> [1] https://bugs.chromium.org/p/chromium/issues/detail?id=803749
>>
>>  drivers/usb/host/ohci-q.c | 17 ++---
>>  1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
>> index b2ec8c399363..4ccb85a67bb3 100644
>> --- a/drivers/usb/host/ohci-q.c
>> +++ b/drivers/usb/host/ohci-q.c
>> @@ -1019,6 +1019,8 @@ static void finish_unlinks(struct ohci_hcd *ohci)
>>  * have modified this list.  normally it's just prepending
>>  * entries (which we'd ignore), but paranoia won't hurt.
>>  */
>> +   *last = ed->ed_next;
>> +   ed->ed_next = NULL;
>> modified = 0;
>>
>> /* unlink urbs as requested, but rescan the list after
>> @@ -1077,21 +1079,22 @@ static void finish_unlinks(struct ohci_hcd *ohci)
>> goto rescan_this;
>>
>> /*
>> -* If no TDs are queued, take ED off the ed_rm_list.
>> +* If no TDs are queued, ED is now idle.
>>  * Otherwise, if the HC is running, reschedule.
>> -* If not, leave it on the list for further dequeues.
>> +* If the HC isn't running, add ED back to the
>> +* start of the list for later processing.
>>  */
>> if (list_empty(>td_list)) {
>> -   *last = ed->ed_next;
>> -   ed->ed_next = NULL;
>> ed->state = ED_IDLE;
>> list_del(>in_use_list);
>> } else if (ohci->rh_state == OHCI_RH_RUNNING) {
>> -   *last = ed->ed_next;
>> -   ed->ed_next = NULL;
>> ed_schedule(ohci, ed);
>> } else {
>> -   last = >ed_next;
>> +   ed->ed_next = ohci->ed_rm_list;
>> +   ohci->ed_rm_list = ed;
>> +   /* Don't loop on the same ED */
>> +   if (last == >ed_rm_list)
>> +   last = >ed_next;
>> }
>>
>> if (modified)
>
> I am experiencing a USB function call hang from userspace with OCHI
> (full speed USB device) after updating from Linux 4.14.15 to 4.14.24
> and noticed this commit.
>
> Here is the Linux 4.14.24 kernel stack trace (extracted from SysRq+w
> and amended with addr2line):
> [] (__schedule) from [] (schedule+0x50/0xb4)
> kernel/sched/core.c:2792
> [] (schedule) from []
> (usb_kill_urb.part.3+0x78/0xa8) 

Re: [v6] usb: ohci: Proper handling of ed_rm_list to handle race condition between usb_kill_urb() and finish_unlinks()

[Jonathan Liu]

Hi,

On 25 March 2018 at 12:21, Jonathan Liu  wrote:
> Hi,
>
> On 8 February 2018 at 14:55, Jeffy Chen  wrote:
>> From: AMAN DEEP 
>>
>> There is a race condition between finish_unlinks->finish_urb() function
>> and usb_kill_urb() in ohci controller case. The finish_urb calls
>> spin_unlock(>lock) before usb_hcd_giveback_urb() function call,
>> then if during this time, usb_kill_urb is called for another endpoint,
>> then new ed will be added to ed_rm_list at beginning for unlink, and
>> ed_rm_list will point to newly added.
>>
>> When finish_urb() is completed in finish_unlinks() and ed->td_list
>> becomes empty as in below code (in finish_unlinks() function):
>>
>> if (list_empty(>td_list)) {
>> *last = ed->ed_next;
>> ed->ed_next = NULL;
>> } else if (ohci->rh_state == OHCI_RH_RUNNING) {
>> *last = ed->ed_next;
>> ed->ed_next = NULL;
>> ed_schedule(ohci, ed);
>> }
>>
>> The *last = ed->ed_next will make ed_rm_list to point to ed->ed_next
>> and previously added ed by usb_kill_urb will be left unreferenced by
>> ed_rm_list. This causes usb_kill_urb() hang forever waiting for
>> finish_unlink to remove added ed from ed_rm_list.
>>
>> The main reason for hang in this race condtion is addition and removal
>> of ed from ed_rm_list in the beginning during usb_kill_urb and later
>> last* is modified in finish_unlinks().
>>
>> As suggested by Alan Stern, the solution for proper handling of
>> ohci->ed_rm_list is to remove ed from the ed_rm_list before finishing
>> any URBs. Then at the end, we can add ed back to the list if necessary.
>>
>> This properly handle the updated ohci->ed_rm_list in usb_kill_urb().
>>
>> Fixes:977dcfdc6031("USB:OHCI:don't lose track of EDs when a controller dies")
>> Acked-by: Alan Stern 
>> CC: 
>> Signed-off-by: Aman Deep 
>> Signed-off-by: Jeffy Chen 
>> ---
>>
>> Changes in v6:
>> This is a resend of Aman Deep's v5 patch [0], which solved the hang we
>> hit [1]. (Thanks Aman :)
>>
>> The v5 has some format issues, so i slightly adjust the commit message.
>>
>> [0] https://www.spinics.net/lists/linux-usb/msg129010.html
>> [1] https://bugs.chromium.org/p/chromium/issues/detail?id=803749
>>
>>  drivers/usb/host/ohci-q.c | 17 ++---
>>  1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
>> index b2ec8c399363..4ccb85a67bb3 100644
>> --- a/drivers/usb/host/ohci-q.c
>> +++ b/drivers/usb/host/ohci-q.c
>> @@ -1019,6 +1019,8 @@ static void finish_unlinks(struct ohci_hcd *ohci)
>>  * have modified this list.  normally it's just prepending
>>  * entries (which we'd ignore), but paranoia won't hurt.
>>  */
>> +   *last = ed->ed_next;
>> +   ed->ed_next = NULL;
>> modified = 0;
>>
>> /* unlink urbs as requested, but rescan the list after
>> @@ -1077,21 +1079,22 @@ static void finish_unlinks(struct ohci_hcd *ohci)
>> goto rescan_this;
>>
>> /*
>> -* If no TDs are queued, take ED off the ed_rm_list.
>> +* If no TDs are queued, ED is now idle.
>>  * Otherwise, if the HC is running, reschedule.
>> -* If not, leave it on the list for further dequeues.
>> +* If the HC isn't running, add ED back to the
>> +* start of the list for later processing.
>>  */
>> if (list_empty(>td_list)) {
>> -   *last = ed->ed_next;
>> -   ed->ed_next = NULL;
>> ed->state = ED_IDLE;
>> list_del(>in_use_list);
>> } else if (ohci->rh_state == OHCI_RH_RUNNING) {
>> -   *last = ed->ed_next;
>> -   ed->ed_next = NULL;
>> ed_schedule(ohci, ed);
>> } else {
>> -   last = >ed_next;
>> +   ed->ed_next = ohci->ed_rm_list;
>> +   ohci->ed_rm_list = ed;
>> +   /* Don't loop on the same ED */
>> +   if (last == >ed_rm_list)
>> +   last = >ed_next;
>> }
>>
>> if (modified)
>
> I am experiencing a USB function call hang from userspace with OCHI
> (full speed USB device) after updating from Linux 4.14.15 to 4.14.24
> and noticed this commit.
>
> Here is the Linux 4.14.24 kernel stack trace (extracted from SysRq+w
> and amended with addr2line):
> [] (__schedule) from [] (schedule+0x50/0xb4)
> kernel/sched/core.c:2792
> [] (schedule) from []
> (usb_kill_urb.part.3+0x78/0xa8) include/asm-generic/preempt.h:59
> [] (usb_kill_urb.part.3) from []
> (usbdev_ioctl+0x1288/0x1cf0) drivers/usb/core/urb.c:690
> [] (usbdev_ioctl) from []
> 

Re: [RFC PATCH 5/6] sched/fair: Select an energy-efficient CPU on task wake-up

[Quentin Perret]

On Wednesday 21 Mar 2018 at 15:35:18 (+), Patrick Bellasi wrote:
> On 20-Mar 09:43, Dietmar Eggemann wrote:
> > From: Quentin Perret 
> 
> [...]
> 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 76bd46502486..65a1bead0773 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -6513,6 +6513,60 @@ static unsigned long compute_energy(struct 
> > task_struct *p, int dst_cpu)
> > return energy;
> >  }
> > 
> > +static bool task_fits(struct task_struct *p, int cpu)
> > +{
> > +   unsigned long next_util = cpu_util_next(cpu, p, cpu);
> > +
> > +   return util_fits_capacity(next_util, capacity_orig_of(cpu));
>  ^
> 
> Since here we are at scheduling CFS tasks, should we not better use
> capacity_of() to account for RT/IRQ pressure ?

Yes, definitely. I change this in v2.

> 
> > +}
> > +
> > +static int find_energy_efficient_cpu(struct sched_domain *sd,
> > +   struct task_struct *p, int prev_cpu)
> > +{
> > +   unsigned long cur_energy, prev_energy, best_energy;
> > +   int cpu, best_cpu = prev_cpu;
> > +
> > +   if (!task_util(p))
> 
> We are still waking up a task... what if the task was previously
> running on a big CPU which is now idle?
> 
> I understand that from a _relative_ energy_diff standpoint there is
> not much to do for a 0 utilization task. However, for those tasks we
> can still try to return the most energy efficient CPU among the ones
> in their cpus_allowed mask.
> 
> It should be a relatively low overhead (maybe contained in a fallback
> most_energy_efficient_cpu() kind of function) which allows, for
> example on ARM big.LITTLE systems, to consolidate those tasks on
> LITTLE CPUs instead for example keep running them on a big CPU.

H so the difficult thing about a task with 0 util is that you don't
know if this is really a small task, or a big task with a very long
period. The only useful thing you know for sure about the task is where
it ran last time, so I guess that makes sense to use that information
rather than make assumptions. There is no perfect solution using the
util_avg of the task.

Now, UTIL_EST is changing the game here. If we use it for task placement
(which I think is the right thing to do), this issue should be a lot
easier to solve. What do you think ?

Thanks,
Quentin

Re: [RFC PATCH 5/6] sched/fair: Select an energy-efficient CPU on task wake-up

[Quentin Perret]

On Wednesday 21 Mar 2018 at 15:35:18 (+), Patrick Bellasi wrote:
> On 20-Mar 09:43, Dietmar Eggemann wrote:
> > From: Quentin Perret 
> 
> [...]
> 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 76bd46502486..65a1bead0773 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -6513,6 +6513,60 @@ static unsigned long compute_energy(struct 
> > task_struct *p, int dst_cpu)
> > return energy;
> >  }
> > 
> > +static bool task_fits(struct task_struct *p, int cpu)
> > +{
> > +   unsigned long next_util = cpu_util_next(cpu, p, cpu);
> > +
> > +   return util_fits_capacity(next_util, capacity_orig_of(cpu));
>  ^
> 
> Since here we are at scheduling CFS tasks, should we not better use
> capacity_of() to account for RT/IRQ pressure ?

Yes, definitely. I change this in v2.

> 
> > +}
> > +
> > +static int find_energy_efficient_cpu(struct sched_domain *sd,
> > +   struct task_struct *p, int prev_cpu)
> > +{
> > +   unsigned long cur_energy, prev_energy, best_energy;
> > +   int cpu, best_cpu = prev_cpu;
> > +
> > +   if (!task_util(p))
> 
> We are still waking up a task... what if the task was previously
> running on a big CPU which is now idle?
> 
> I understand that from a _relative_ energy_diff standpoint there is
> not much to do for a 0 utilization task. However, for those tasks we
> can still try to return the most energy efficient CPU among the ones
> in their cpus_allowed mask.
> 
> It should be a relatively low overhead (maybe contained in a fallback
> most_energy_efficient_cpu() kind of function) which allows, for
> example on ARM big.LITTLE systems, to consolidate those tasks on
> LITTLE CPUs instead for example keep running them on a big CPU.

H so the difficult thing about a task with 0 util is that you don't
know if this is really a small task, or a big task with a very long
period. The only useful thing you know for sure about the task is where
it ran last time, so I guess that makes sense to use that information
rather than make assumptions. There is no perfect solution using the
util_avg of the task.

Now, UTIL_EST is changing the game here. If we use it for task placement
(which I think is the right thing to do), this issue should be a lot
easier to solve. What do you think ?

Thanks,
Quentin

Re: [RFC PATCH 5/6] sched/fair: Select an energy-efficient CPU on task wake-up

[Quentin Perret]

On Friday 23 Mar 2018 at 16:00:59 (+), Morten Rasmussen wrote:
> On Thu, Mar 22, 2018 at 09:27:43AM -0700, Joel Fernandes wrote:
> > Hi,
> > 
> > On Tue, Mar 20, 2018 at 2:43 AM, Dietmar Eggemann
> >  wrote:

[...]

> > Is it possible that before the wakeup, the task's affinity is changed
> > so that p->cpus_allowed no longer contains prev_cpu ? In that case
> > prev_energy wouldn't matter since previous CPU is no longer an option?
> 
> It is possible to wake-up with a disallowed prev_cpu. In fact
> select_idle_sibling() may happily return a disallowed cpu in that case.
> The mistake gets fixed in select_task_rq() which uses
> select_fallback_rq() to find an allowed cpu instead.
> 
> Could we fix the issue in find_energy_efficient_cpu() by a simple test
> like below
> 
> if (cpumask_test_cpu(prev_cpu, >cpus_allowed))
>   prev_energy = best_energy = compute_energy(p, prev_cpu);
> else
>   prev_energy = best_energy = ULONG_MAX;

Right, that should work. I'll change this in v2.

Thanks,
Quentin

Re: [RFC PATCH 5/6] sched/fair: Select an energy-efficient CPU on task wake-up

[Quentin Perret]

On Friday 23 Mar 2018 at 16:00:59 (+), Morten Rasmussen wrote:
> On Thu, Mar 22, 2018 at 09:27:43AM -0700, Joel Fernandes wrote:
> > Hi,
> > 
> > On Tue, Mar 20, 2018 at 2:43 AM, Dietmar Eggemann
> >  wrote:

[...]

> > Is it possible that before the wakeup, the task's affinity is changed
> > so that p->cpus_allowed no longer contains prev_cpu ? In that case
> > prev_energy wouldn't matter since previous CPU is no longer an option?
> 
> It is possible to wake-up with a disallowed prev_cpu. In fact
> select_idle_sibling() may happily return a disallowed cpu in that case.
> The mistake gets fixed in select_task_rq() which uses
> select_fallback_rq() to find an allowed cpu instead.
> 
> Could we fix the issue in find_energy_efficient_cpu() by a simple test
> like below
> 
> if (cpumask_test_cpu(prev_cpu, >cpus_allowed))
>   prev_energy = best_energy = compute_energy(p, prev_cpu);
> else
>   prev_energy = best_energy = ULONG_MAX;

Right, that should work. I'll change this in v2.

Thanks,
Quentin

Re: [PATCH 6/8] Pmalloc selftest

[Igor Stoppa]

On 14/03/18 14:25, Matthew Wilcox wrote:
> On Tue, Mar 13, 2018 at 11:45:52PM +0200, Igor Stoppa wrote:
>> Add basic self-test functionality for pmalloc.
> 
> Here're some additional tests for your test-suite:
> 
>   for (i = 1; i; i *= 2)
>   pzalloc(pool, i - 1, GFP_KERNEL);
> 

Ok, I have almost finished the rewrite.
I still have to address this comment.

When I run the test, eventually the system runs out of memory, it keeps
getting allocation errors from vmalloc, until i finally overflows and
becomes 0.

Am I supposed to do something about it?
If pmalloc receives a request that the vmalloc backend cannot satisfy, I
would prefer that vmalloc itself produces the warning and pmalloc
returns NULL.

This doesn't look like a test case that one can leave always enabled in
a build, but maybe I'm missing the point.

--
igor

Re: [PATCH 6/8] Pmalloc selftest

[Igor Stoppa]

On 14/03/18 14:25, Matthew Wilcox wrote:
> On Tue, Mar 13, 2018 at 11:45:52PM +0200, Igor Stoppa wrote:
>> Add basic self-test functionality for pmalloc.
> 
> Here're some additional tests for your test-suite:
> 
>   for (i = 1; i; i *= 2)
>   pzalloc(pool, i - 1, GFP_KERNEL);
> 

Ok, I have almost finished the rewrite.
I still have to address this comment.

When I run the test, eventually the system runs out of memory, it keeps
getting allocation errors from vmalloc, until i finally overflows and
becomes 0.

Am I supposed to do something about it?
If pmalloc receives a request that the vmalloc backend cannot satisfy, I
would prefer that vmalloc itself produces the warning and pmalloc
returns NULL.

This doesn't look like a test case that one can leave always enabled in
a build, but maybe I'm missing the point.

--
igor

[PATCH v2 2/2] drivers: soc: Add LLCC driver

[Rishabh Bhatnagar]

LLCC (Last Level Cache Controller) provides additional cache memory
in the system. LLCC is partitioned into muliple slices and each
slice getting its own priority, size, ID and other config parameters.
LLCC driver programs these parameters for each slice. Clients that
are assigned to use LLCC need to get information such size & ID of the
 slice they get and activate or deactivate the slice as needed. LLCC driver
provides API interfaces for the clients to perform these operations.

Change-Id: I93c5f3225d0a917f0d6c64c7588a4e64e33a59ed
Signed-off-by: Channagoud Kadabi 
Signed-off-by: Rishabh Bhatnagar 
---
 drivers/soc/qcom/Kconfig   |  16 ++
 drivers/soc/qcom/Makefile  |   2 +
 drivers/soc/qcom/llcc-sdm845.c | 120 ++
 drivers/soc/qcom/llcc-slice.c  | 454 +
 include/linux/soc/qcom/llcc-qcom.h | 178 +++
 5 files changed, 770 insertions(+)
 create mode 100644 drivers/soc/qcom/llcc-sdm845.c
 create mode 100644 drivers/soc/qcom/llcc-slice.c
 create mode 100644 include/linux/soc/qcom/llcc-qcom.h

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index e050eb8..28237fc 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -21,6 +21,22 @@ config QCOM_GSBI
   functions for connecting the underlying serial UART, SPI, and I2C
   devices to the output pins.
 
+config QCOM_LLCC
+   tristate "Qualcomm Technologies, Inc. LLCC driver"
+   depends on ARCH_QCOM
+   help
+ Qualcomm Technologies, Inc. platform specific LLCC driver for Last
+ Level Cache. This provides interfaces to client's that use the LLCC.
+ Say yes here to enable LLCC slice driver.
+
+config QCOM_SDM845_LLCC
+   tristate "Qualcomm Technologies, Inc. SDM845 LLCC driver"
+   depends on QCOM_LLCC
+   help
+ Say yes here to enable the LLCC driver for SDM845. This is provides
+ data required to configure LLCC so that clients can start using the
+ LLCC slices.
+
 config QCOM_MDT_LOADER
tristate
select QCOM_SCM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index dcebf28..234560f 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -12,3 +12,5 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 obj-$(CONFIG_QCOM_SMP2P)   += smp2p.o
 obj-$(CONFIG_QCOM_SMSM)+= smsm.o
 obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
+obj-$(CONFIG_QCOM_LLCC) += llcc-core.o llcc-slice.o
+obj-$(CONFIG_QCOM_SDM845_LLCC) += llcc-sdm845.o
diff --git a/drivers/soc/qcom/llcc-sdm845.c b/drivers/soc/qcom/llcc-sdm845.c
new file mode 100644
index 000..cd431d9
--- /dev/null
+++ b/drivers/soc/qcom/llcc-sdm845.c
@@ -0,0 +1,120 @@
+/* Copyright (c) 2016-2017, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only 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 
+
+
+/*
+ * SCT entry contains of the following parameters
+ * name: Name of the client's use case for which the llcc slice is used
+ * uid: Unique id for the client's use case
+ * slice_id: llcc slice id for each client
+ * max_cap: The maximum capacity of the cache slice provided in KB
+ * priority: Priority of the client used to select victim line for replacement
+ * fixed_size: Determine of the slice has a fixed capacity
+ * bonus_ways: Bonus ways to be used by any slice, bonus way is used only if
+ * it't not a reserved way.
+ * res_ways: Reserved ways for the cache slice, the reserved ways cannot be 
+ *  used by any other client than the one its assigned to.
+ * cache_mode: Each slice operates as a cache, this controls the mode of the
+ * slice normal or TCM
+ * probe_target_ways: Determines what ways to probe for access hit. When
+ *configured to 1 only bonus and reseved ways are probed.
+ *when configured to 0 all ways in llcc are probed.
+ * dis_cap_alloc: Disable capacity based allocation for a client
+ * retain_on_pc: If this bit is set and client has maitained active vote
+ *   then the ways assigned to this client are not flushed on power
+ *   collapse.
+ * activate_on_init: Activate the slice immidiately after the SCT is programmed
+ */
+#define SCT_ENTRY(n, uid, sid, mc, p, fs, bway, rway, cmod, ptw, dca, rp, a) \
+   {   \
+   .name = n,  \
+  

[PATCH v2 0/2] SDM845 System Cache Driver

[Rishabh Bhatnagar]

This series implements system cache or LLCC(Last Level Cache Controller) driver
for SDM845 SOC. The purpose of the driver is to partition the system cache and
program the settings such as priortiy, lines to probe while doing a look up
in the system cache, low power related settings etc. The partitions are called
cache slices. Each cache slice is associated with size and SCID(System Cache ID)
The driver also provides API for clients to query the cache slice details,
activate and deactivate them.

The driver can be broadly classified into:
* SOC specific driver: llcc-sdm845.c: Cache partitioning and cache slice
properties for usecases on sdm845 that need to use system cache.

* API : llcc-slice.c: Exports APIs to clients to query cache slice details,
activate and deactivate cache slices. Register the child devices using platform 
APIs.

Changes since v1:
* Added Makefile and Kconfig.

ckad...@codeaurora.org (2):
  Documentation: Documentaion for qcom, llcc
  drivers: soc: Add LLCC driver

 .../devicetree/bindings/arm/msm/qcom,llcc.txt  |  70 
 drivers/soc/qcom/Kconfig   |  16 +
 drivers/soc/qcom/Makefile  |   2 +
 drivers/soc/qcom/llcc-sdm845.c | 120 ++
 drivers/soc/qcom/llcc-slice.c  | 454 +
 include/linux/soc/qcom/llcc-qcom.h | 178 
 6 files changed, 840 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
 create mode 100644 drivers/soc/qcom/llcc-sdm845.c
 create mode 100644 drivers/soc/qcom/llcc-slice.c
 create mode 100644 include/linux/soc/qcom/llcc-qcom.h

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

[PATCH v2 0/2] SDM845 System Cache Driver

[Rishabh Bhatnagar]

This series implements system cache or LLCC(Last Level Cache Controller) driver
for SDM845 SOC. The purpose of the driver is to partition the system cache and
program the settings such as priortiy, lines to probe while doing a look up
in the system cache, low power related settings etc. The partitions are called
cache slices. Each cache slice is associated with size and SCID(System Cache ID)
The driver also provides API for clients to query the cache slice details,
activate and deactivate them.

The driver can be broadly classified into:
* SOC specific driver: llcc-sdm845.c: Cache partitioning and cache slice
properties for usecases on sdm845 that need to use system cache.

* API : llcc-slice.c: Exports APIs to clients to query cache slice details,
activate and deactivate cache slices. Register the child devices using platform 
APIs.

Changes since v1:
* Added Makefile and Kconfig.

ckad...@codeaurora.org (2):
  Documentation: Documentaion for qcom, llcc
  drivers: soc: Add LLCC driver

 .../devicetree/bindings/arm/msm/qcom,llcc.txt  |  70 
 drivers/soc/qcom/Kconfig   |  16 +
 drivers/soc/qcom/Makefile  |   2 +
 drivers/soc/qcom/llcc-sdm845.c | 120 ++
 drivers/soc/qcom/llcc-slice.c  | 454 +
 include/linux/soc/qcom/llcc-qcom.h | 178 
 6 files changed, 840 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
 create mode 100644 drivers/soc/qcom/llcc-sdm845.c
 create mode 100644 drivers/soc/qcom/llcc-slice.c
 create mode 100644 include/linux/soc/qcom/llcc-qcom.h

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

[PATCH v2 2/2] drivers: soc: Add LLCC driver

[Rishabh Bhatnagar]

LLCC (Last Level Cache Controller) provides additional cache memory
in the system. LLCC is partitioned into muliple slices and each
slice getting its own priority, size, ID and other config parameters.
LLCC driver programs these parameters for each slice. Clients that
are assigned to use LLCC need to get information such size & ID of the
 slice they get and activate or deactivate the slice as needed. LLCC driver
provides API interfaces for the clients to perform these operations.

Change-Id: I93c5f3225d0a917f0d6c64c7588a4e64e33a59ed
Signed-off-by: Channagoud Kadabi 
Signed-off-by: Rishabh Bhatnagar 
---
 drivers/soc/qcom/Kconfig   |  16 ++
 drivers/soc/qcom/Makefile  |   2 +
 drivers/soc/qcom/llcc-sdm845.c | 120 ++
 drivers/soc/qcom/llcc-slice.c  | 454 +
 include/linux/soc/qcom/llcc-qcom.h | 178 +++
 5 files changed, 770 insertions(+)
 create mode 100644 drivers/soc/qcom/llcc-sdm845.c
 create mode 100644 drivers/soc/qcom/llcc-slice.c
 create mode 100644 include/linux/soc/qcom/llcc-qcom.h

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index e050eb8..28237fc 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -21,6 +21,22 @@ config QCOM_GSBI
   functions for connecting the underlying serial UART, SPI, and I2C
   devices to the output pins.
 
+config QCOM_LLCC
+   tristate "Qualcomm Technologies, Inc. LLCC driver"
+   depends on ARCH_QCOM
+   help
+ Qualcomm Technologies, Inc. platform specific LLCC driver for Last
+ Level Cache. This provides interfaces to client's that use the LLCC.
+ Say yes here to enable LLCC slice driver.
+
+config QCOM_SDM845_LLCC
+   tristate "Qualcomm Technologies, Inc. SDM845 LLCC driver"
+   depends on QCOM_LLCC
+   help
+ Say yes here to enable the LLCC driver for SDM845. This is provides
+ data required to configure LLCC so that clients can start using the
+ LLCC slices.
+
 config QCOM_MDT_LOADER
tristate
select QCOM_SCM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index dcebf28..234560f 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -12,3 +12,5 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 obj-$(CONFIG_QCOM_SMP2P)   += smp2p.o
 obj-$(CONFIG_QCOM_SMSM)+= smsm.o
 obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
+obj-$(CONFIG_QCOM_LLCC) += llcc-core.o llcc-slice.o
+obj-$(CONFIG_QCOM_SDM845_LLCC) += llcc-sdm845.o
diff --git a/drivers/soc/qcom/llcc-sdm845.c b/drivers/soc/qcom/llcc-sdm845.c
new file mode 100644
index 000..cd431d9
--- /dev/null
+++ b/drivers/soc/qcom/llcc-sdm845.c
@@ -0,0 +1,120 @@
+/* Copyright (c) 2016-2017, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only 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 
+
+
+/*
+ * SCT entry contains of the following parameters
+ * name: Name of the client's use case for which the llcc slice is used
+ * uid: Unique id for the client's use case
+ * slice_id: llcc slice id for each client
+ * max_cap: The maximum capacity of the cache slice provided in KB
+ * priority: Priority of the client used to select victim line for replacement
+ * fixed_size: Determine of the slice has a fixed capacity
+ * bonus_ways: Bonus ways to be used by any slice, bonus way is used only if
+ * it't not a reserved way.
+ * res_ways: Reserved ways for the cache slice, the reserved ways cannot be 
+ *  used by any other client than the one its assigned to.
+ * cache_mode: Each slice operates as a cache, this controls the mode of the
+ * slice normal or TCM
+ * probe_target_ways: Determines what ways to probe for access hit. When
+ *configured to 1 only bonus and reseved ways are probed.
+ *when configured to 0 all ways in llcc are probed.
+ * dis_cap_alloc: Disable capacity based allocation for a client
+ * retain_on_pc: If this bit is set and client has maitained active vote
+ *   then the ways assigned to this client are not flushed on power
+ *   collapse.
+ * activate_on_init: Activate the slice immidiately after the SCT is programmed
+ */
+#define SCT_ENTRY(n, uid, sid, mc, p, fs, bway, rway, cmod, ptw, dca, rp, a) \
+   {   \
+   .name = n,  \
+   .usecase_id = uid,  \
+ 

[PATCH v2 1/2] Documentation: Documentation for qcom, llcc

[Rishabh Bhatnagar]

Documentation for last level cache controller device tree bindings,
client bindings usage examples.

Change-Id: Ic2d6d6154ab8269cfce6828e9f2250320a0572e8
Signed-off-by: Channagoud Kadabi 
Signed-off-by: Rishabh Bhatnagar 
---
 .../devicetree/bindings/arm/msm/qcom,llcc.txt  | 70 ++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt

diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt 
b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
new file mode 100644
index 000..ceb20a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
@@ -0,0 +1,70 @@
+== Introduction==
+
+LLCC (Last Level Cache Controller) driver is implemented as a platform device.
+The driver Programs the SCT (system configuration table). The SCT programming 
+divides the system cache into slices. Each slice is assigned an ID A.K.A 
+SCID(Sub-cache ID).
+HW modules that are designated to use the system cache are known as clients.   
+Each client must also be represented as a node in the device tree just like
+any other hw module.   
+One client can have multiple SCID's assigned meaning each client could get 

+multiple slices in the cache. Client can use the slices for various pre-defined
+usecases. Each client defines a set of names for these usecases in its 
+device tree binding.   
+Client makes a request to LLCC device to get cache-slices properties for each  
+of its usecase. Client gets the information like cache slice ID and size of the
+cache slice.
+
+== llcc device ==
+
+Properties:
+- compatible:
+Usage: required
+Value type: 
+Definition: must be "qcom,sdm855-llcc"
+
+- reg:
+Usage: required
+Value Type: 
+Definition: must be addresses and sizes of the LLCC registers
+
+- #cache-cells:
+Usage: required
+Value Type: 
+Definition: Number of cache cells, must be 1
+
+- max-slices:
+usage: required
+Value Type: 
+Definition: Number of cache slices supported by hardware
+
+Example:
+
+   llcc: qcom,sdm855-llcc@0110 {
+   compatible = "qcom,sdm845-llcc";
+   reg = <0x0110 0x25>;
+   #cache-cells = <1>;
+   max-slices = <32>;
+   };
+
+== Client ==
+
+Required properties:
+- cache-slice-names:
+Usage: required
+Value type: 
+Definition: A set of names that identify the usecase names of a client 
that
+   uses cache slice. These strings are used to look up the 
cache slice
+   entries by name.
+
+- cache-slices:
+Usage: required
+Value type: 
+Definition: The tuple has phandle to llcc device as the first argument 
and 
+   the second argument is the usecase id of the client.
+For Example:
+
+venus {
+   cache-slice-names = "vidsc0", "vidsc1";
+   cache-slices = < 2>, < 3>;
+};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 1/2] Documentation: Documentation for qcom, llcc

[Rishabh Bhatnagar]

Documentation for last level cache controller device tree bindings,
client bindings usage examples.

Change-Id: Ic2d6d6154ab8269cfce6828e9f2250320a0572e8
Signed-off-by: Channagoud Kadabi 
Signed-off-by: Rishabh Bhatnagar 
---
 .../devicetree/bindings/arm/msm/qcom,llcc.txt  | 70 ++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt

diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt 
b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
new file mode 100644
index 000..ceb20a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
@@ -0,0 +1,70 @@
+== Introduction==
+
+LLCC (Last Level Cache Controller) driver is implemented as a platform device.
+The driver Programs the SCT (system configuration table). The SCT programming 
+divides the system cache into slices. Each slice is assigned an ID A.K.A 
+SCID(Sub-cache ID).
+HW modules that are designated to use the system cache are known as clients.   
+Each client must also be represented as a node in the device tree just like
+any other hw module.   
+One client can have multiple SCID's assigned meaning each client could get 

+multiple slices in the cache. Client can use the slices for various pre-defined
+usecases. Each client defines a set of names for these usecases in its 
+device tree binding.   
+Client makes a request to LLCC device to get cache-slices properties for each  
+of its usecase. Client gets the information like cache slice ID and size of the
+cache slice.
+
+== llcc device ==
+
+Properties:
+- compatible:
+Usage: required
+Value type: 
+Definition: must be "qcom,sdm855-llcc"
+
+- reg:
+Usage: required
+Value Type: 
+Definition: must be addresses and sizes of the LLCC registers
+
+- #cache-cells:
+Usage: required
+Value Type: 
+Definition: Number of cache cells, must be 1
+
+- max-slices:
+usage: required
+Value Type: 
+Definition: Number of cache slices supported by hardware
+
+Example:
+
+   llcc: qcom,sdm855-llcc@0110 {
+   compatible = "qcom,sdm845-llcc";
+   reg = <0x0110 0x25>;
+   #cache-cells = <1>;
+   max-slices = <32>;
+   };
+
+== Client ==
+
+Required properties:
+- cache-slice-names:
+Usage: required
+Value type: 
+Definition: A set of names that identify the usecase names of a client 
that
+   uses cache slice. These strings are used to look up the 
cache slice
+   entries by name.
+
+- cache-slices:
+Usage: required
+Value type: 
+Definition: The tuple has phandle to llcc device as the first argument 
and 
+   the second argument is the usecase id of the client.
+For Example:
+
+venus {
+   cache-slice-names = "vidsc0", "vidsc1";
+   cache-slices = < 2>, < 3>;
+};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[GIT PULL] Revert "mqueue: switch to on-demand creation of internal mount"

[Eric W. Biederman]

Linus,

Please pull the for-linus branch from the git tree:

   git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git 
for-linus

   HEAD: cfb2f6f6e0ba11ea7b263d6b69c170c4b32ac0ea Revert "mqueue: switch to 
on-demand creation of internal mount"

This fixes a regression that came in the merge window for v4.16.  The
problem is that the permissions for mounting and using the mqueuefs
filesystem are broken.  The necessary permission check is missing
letting people who should not be able to mount mqueuefs mount mqueuefs.
The field sb->s_user_ns is set incorrectly not allowing the mounter of
mqueuefs to remount and otherwise have proper control over the
filesystem.

Al Viro and I see the path to the necessary fixes differently and I am
not even certain at this point he actually sees all of the necessary
fixes.  Given a couple weeks we can probably work something out but I
don't see the review being resolved in time for the final v4.16.  I
don't want v4.16 shipping with a nasty regression.  So unfortunately I
am sending a revert.

Eric

From: "Eric W. Biederman" 
Date: Sat, 24 Mar 2018 11:28:14 -0500
Subject: [PATCH] Revert "mqueue: switch to on-demand creation of internal mount"

This reverts commit 36735a6a2b5e042db1af956ce4bcc13f3ff99e21.

Aleksa Sarai  writes:
> [REGRESSION v4.16-rc6] [PATCH] mqueue: forbid unprivileged user access to 
> internal mount
>
> Felix reported weird behaviour on 4.16.0-rc6 with regards to mqueue[1],
> which was introduced by 36735a6a2b5e ("mqueue: switch to on-demand
> creation of internal mount").
>
> Basically, the reproducer boils down to being able to mount mqueue if
> you create a new user namespace, even if you don't unshare the IPC
> namespace.
>
> Previously this was not possible, and you would get an -EPERM. The mount
> is the *host* mqueue mount, which is being cached and just returned from
> mqueue_mount(). To be honest, I'm not sure if this is safe or not (or if
> it was intentional -- since I'm not familiar with mqueue).
>
> To me it looks like there is a missing permission check. I've included a
> patch below that I've compile-tested, and should block the above case.
> Can someone please tell me if I'm missing something? Is this actually
> safe?
>
> [1]: https://github.com/docker/docker/issues/36674

The issue is a lot deeper than a missing permission check.  sb->s_user_ns
was is improperly set as well.  So in addition to the filesystem being
mounted when it should not be mounted, so things are not allow that should
be.

We are practically to the release of 4.16 and there is no agreement between
Al Viro and myself on what the code should looks like to fix things properly.
So revert the code to what it was before so that we can take our time
and discuss this properly.

Fixes: 36735a6a2b5e ("mqueue: switch to on-demand creation of internal mount")
Reported-by: Felix Abecassis 
Reported-by: Aleksa Sarai 
Signed-off-by: "Eric W. Biederman" 
---
 ipc/mqueue.c | 74 
 1 file changed, 19 insertions(+), 55 deletions(-)

diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index d7f309f74dec..a808f29d4c5a 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -325,9 +325,8 @@ static struct inode *mqueue_get_inode(struct super_block 
*sb,
 static int mqueue_fill_super(struct super_block *sb, void *data, int silent)
 {
struct inode *inode;
-   struct ipc_namespace *ns = data;
+   struct ipc_namespace *ns = sb->s_fs_info;
 
-   sb->s_fs_info = ns;
sb->s_iflags |= SB_I_NOEXEC | SB_I_NODEV;
sb->s_blocksize = PAGE_SIZE;
sb->s_blocksize_bits = PAGE_SHIFT;
@@ -344,44 +343,18 @@ static int mqueue_fill_super(struct super_block *sb, void 
*data, int silent)
return 0;
 }
 
-static struct file_system_type mqueue_fs_type;
-/*
- * Return value is pinned only by reference in ->mq_mnt; it will
- * live until ipcns dies.  Caller does not need to drop it.
- */
-static struct vfsmount *mq_internal_mount(void)
-{
-   struct ipc_namespace *ns = current->nsproxy->ipc_ns;
-   struct vfsmount *m = ns->mq_mnt;
-   if (m)
-   return m;
-   m = kern_mount_data(_fs_type, ns);
-   spin_lock(_lock);
-   if (unlikely(ns->mq_mnt)) {
-   spin_unlock(_lock);
-   if (!IS_ERR(m))
-   kern_unmount(m);
-   return ns->mq_mnt;
-   }
-   if (!IS_ERR(m))
-   ns->mq_mnt = m;
-   spin_unlock(_lock);
-   return m;
-}
-
 static struct dentry *mqueue_mount(struct file_system_type *fs_type,
 int flags, const char *dev_name,
 void *data)
 {
-   struct vfsmount *m;
-   if (flags & SB_KERNMOUNT)
-   return mount_nodev(fs_type, flags, data, mqueue_fill_super);
-   m = mq_internal_mount();
-   if (IS_ERR(m))
-   

[GIT PULL] Revert "mqueue: switch to on-demand creation of internal mount"

[Eric W. Biederman]

Linus,

Please pull the for-linus branch from the git tree:

   git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git 
for-linus

   HEAD: cfb2f6f6e0ba11ea7b263d6b69c170c4b32ac0ea Revert "mqueue: switch to 
on-demand creation of internal mount"

This fixes a regression that came in the merge window for v4.16.  The
problem is that the permissions for mounting and using the mqueuefs
filesystem are broken.  The necessary permission check is missing
letting people who should not be able to mount mqueuefs mount mqueuefs.
The field sb->s_user_ns is set incorrectly not allowing the mounter of
mqueuefs to remount and otherwise have proper control over the
filesystem.

Al Viro and I see the path to the necessary fixes differently and I am
not even certain at this point he actually sees all of the necessary
fixes.  Given a couple weeks we can probably work something out but I
don't see the review being resolved in time for the final v4.16.  I
don't want v4.16 shipping with a nasty regression.  So unfortunately I
am sending a revert.

Eric

From: "Eric W. Biederman" 
Date: Sat, 24 Mar 2018 11:28:14 -0500
Subject: [PATCH] Revert "mqueue: switch to on-demand creation of internal mount"

This reverts commit 36735a6a2b5e042db1af956ce4bcc13f3ff99e21.

Aleksa Sarai  writes:
> [REGRESSION v4.16-rc6] [PATCH] mqueue: forbid unprivileged user access to 
> internal mount
>
> Felix reported weird behaviour on 4.16.0-rc6 with regards to mqueue[1],
> which was introduced by 36735a6a2b5e ("mqueue: switch to on-demand
> creation of internal mount").
>
> Basically, the reproducer boils down to being able to mount mqueue if
> you create a new user namespace, even if you don't unshare the IPC
> namespace.
>
> Previously this was not possible, and you would get an -EPERM. The mount
> is the *host* mqueue mount, which is being cached and just returned from
> mqueue_mount(). To be honest, I'm not sure if this is safe or not (or if
> it was intentional -- since I'm not familiar with mqueue).
>
> To me it looks like there is a missing permission check. I've included a
> patch below that I've compile-tested, and should block the above case.
> Can someone please tell me if I'm missing something? Is this actually
> safe?
>
> [1]: https://github.com/docker/docker/issues/36674

The issue is a lot deeper than a missing permission check.  sb->s_user_ns
was is improperly set as well.  So in addition to the filesystem being
mounted when it should not be mounted, so things are not allow that should
be.

We are practically to the release of 4.16 and there is no agreement between
Al Viro and myself on what the code should looks like to fix things properly.
So revert the code to what it was before so that we can take our time
and discuss this properly.

Fixes: 36735a6a2b5e ("mqueue: switch to on-demand creation of internal mount")
Reported-by: Felix Abecassis 
Reported-by: Aleksa Sarai 
Signed-off-by: "Eric W. Biederman" 
---
 ipc/mqueue.c | 74 
 1 file changed, 19 insertions(+), 55 deletions(-)

diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index d7f309f74dec..a808f29d4c5a 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -325,9 +325,8 @@ static struct inode *mqueue_get_inode(struct super_block 
*sb,
 static int mqueue_fill_super(struct super_block *sb, void *data, int silent)
 {
struct inode *inode;
-   struct ipc_namespace *ns = data;
+   struct ipc_namespace *ns = sb->s_fs_info;
 
-   sb->s_fs_info = ns;
sb->s_iflags |= SB_I_NOEXEC | SB_I_NODEV;
sb->s_blocksize = PAGE_SIZE;
sb->s_blocksize_bits = PAGE_SHIFT;
@@ -344,44 +343,18 @@ static int mqueue_fill_super(struct super_block *sb, void 
*data, int silent)
return 0;
 }
 
-static struct file_system_type mqueue_fs_type;
-/*
- * Return value is pinned only by reference in ->mq_mnt; it will
- * live until ipcns dies.  Caller does not need to drop it.
- */
-static struct vfsmount *mq_internal_mount(void)
-{
-   struct ipc_namespace *ns = current->nsproxy->ipc_ns;
-   struct vfsmount *m = ns->mq_mnt;
-   if (m)
-   return m;
-   m = kern_mount_data(_fs_type, ns);
-   spin_lock(_lock);
-   if (unlikely(ns->mq_mnt)) {
-   spin_unlock(_lock);
-   if (!IS_ERR(m))
-   kern_unmount(m);
-   return ns->mq_mnt;
-   }
-   if (!IS_ERR(m))
-   ns->mq_mnt = m;
-   spin_unlock(_lock);
-   return m;
-}
-
 static struct dentry *mqueue_mount(struct file_system_type *fs_type,
 int flags, const char *dev_name,
 void *data)
 {
-   struct vfsmount *m;
-   if (flags & SB_KERNMOUNT)
-   return mount_nodev(fs_type, flags, data, mqueue_fill_super);
-   m = mq_internal_mount();
-   if (IS_ERR(m))
-   return ERR_CAST(m);
-   atomic_inc(>mnt_sb->s_active);
-   down_write(>mnt_sb->s_umount);
- 

Re: [v6] usb: ohci: Proper handling of ed_rm_list to handle race condition between usb_kill_urb() and finish_unlinks()

[Jonathan Liu]

Hi,

On 8 February 2018 at 14:55, Jeffy Chen  wrote:
> From: AMAN DEEP 
>
> There is a race condition between finish_unlinks->finish_urb() function
> and usb_kill_urb() in ohci controller case. The finish_urb calls
> spin_unlock(>lock) before usb_hcd_giveback_urb() function call,
> then if during this time, usb_kill_urb is called for another endpoint,
> then new ed will be added to ed_rm_list at beginning for unlink, and
> ed_rm_list will point to newly added.
>
> When finish_urb() is completed in finish_unlinks() and ed->td_list
> becomes empty as in below code (in finish_unlinks() function):
>
> if (list_empty(>td_list)) {
> *last = ed->ed_next;
> ed->ed_next = NULL;
> } else if (ohci->rh_state == OHCI_RH_RUNNING) {
> *last = ed->ed_next;
> ed->ed_next = NULL;
> ed_schedule(ohci, ed);
> }
>
> The *last = ed->ed_next will make ed_rm_list to point to ed->ed_next
> and previously added ed by usb_kill_urb will be left unreferenced by
> ed_rm_list. This causes usb_kill_urb() hang forever waiting for
> finish_unlink to remove added ed from ed_rm_list.
>
> The main reason for hang in this race condtion is addition and removal
> of ed from ed_rm_list in the beginning during usb_kill_urb and later
> last* is modified in finish_unlinks().
>
> As suggested by Alan Stern, the solution for proper handling of
> ohci->ed_rm_list is to remove ed from the ed_rm_list before finishing
> any URBs. Then at the end, we can add ed back to the list if necessary.
>
> This properly handle the updated ohci->ed_rm_list in usb_kill_urb().
>
> Fixes:977dcfdc6031("USB:OHCI:don't lose track of EDs when a controller dies")
> Acked-by: Alan Stern 
> CC: 
> Signed-off-by: Aman Deep 
> Signed-off-by: Jeffy Chen 
> ---
>
> Changes in v6:
> This is a resend of Aman Deep's v5 patch [0], which solved the hang we
> hit [1]. (Thanks Aman :)
>
> The v5 has some format issues, so i slightly adjust the commit message.
>
> [0] https://www.spinics.net/lists/linux-usb/msg129010.html
> [1] https://bugs.chromium.org/p/chromium/issues/detail?id=803749
>
>  drivers/usb/host/ohci-q.c | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
> index b2ec8c399363..4ccb85a67bb3 100644
> --- a/drivers/usb/host/ohci-q.c
> +++ b/drivers/usb/host/ohci-q.c
> @@ -1019,6 +1019,8 @@ static void finish_unlinks(struct ohci_hcd *ohci)
>  * have modified this list.  normally it's just prepending
>  * entries (which we'd ignore), but paranoia won't hurt.
>  */
> +   *last = ed->ed_next;
> +   ed->ed_next = NULL;
> modified = 0;
>
> /* unlink urbs as requested, but rescan the list after
> @@ -1077,21 +1079,22 @@ static void finish_unlinks(struct ohci_hcd *ohci)
> goto rescan_this;
>
> /*
> -* If no TDs are queued, take ED off the ed_rm_list.
> +* If no TDs are queued, ED is now idle.
>  * Otherwise, if the HC is running, reschedule.
> -* If not, leave it on the list for further dequeues.
> +* If the HC isn't running, add ED back to the
> +* start of the list for later processing.
>  */
> if (list_empty(>td_list)) {
> -   *last = ed->ed_next;
> -   ed->ed_next = NULL;
> ed->state = ED_IDLE;
> list_del(>in_use_list);
> } else if (ohci->rh_state == OHCI_RH_RUNNING) {
> -   *last = ed->ed_next;
> -   ed->ed_next = NULL;
> ed_schedule(ohci, ed);
> } else {
> -   last = >ed_next;
> +   ed->ed_next = ohci->ed_rm_list;
> +   ohci->ed_rm_list = ed;
> +   /* Don't loop on the same ED */
> +   if (last == >ed_rm_list)
> +   last = >ed_next;
> }
>
> if (modified)

I am experiencing a USB function call hang from userspace with OCHI
(full speed USB device) after updating from Linux 4.14.15 to 4.14.24
and noticed this commit.

Here is the Linux 4.14.24 kernel stack trace (extracted from SysRq+w
and amended with addr2line):
[] (__schedule) from [] (schedule+0x50/0xb4)
kernel/sched/core.c:2792
[] (schedule) from []
(usb_kill_urb.part.3+0x78/0xa8) include/asm-generic/preempt.h:59
[] (usb_kill_urb.part.3) from []
(usbdev_ioctl+0x1288/0x1cf0) drivers/usb/core/urb.c:690
[] (usbdev_ioctl) from []
(do_vfs_ioctl+0x9c/0x8ec) drivers/usb/core/devio.c:1835

Re: [v6] usb: ohci: Proper handling of ed_rm_list to handle race condition between usb_kill_urb() and finish_unlinks()

[Jonathan Liu]

Hi,

On 8 February 2018 at 14:55, Jeffy Chen  wrote:
> From: AMAN DEEP 
>
> There is a race condition between finish_unlinks->finish_urb() function
> and usb_kill_urb() in ohci controller case. The finish_urb calls
> spin_unlock(>lock) before usb_hcd_giveback_urb() function call,
> then if during this time, usb_kill_urb is called for another endpoint,
> then new ed will be added to ed_rm_list at beginning for unlink, and
> ed_rm_list will point to newly added.
>
> When finish_urb() is completed in finish_unlinks() and ed->td_list
> becomes empty as in below code (in finish_unlinks() function):
>
> if (list_empty(>td_list)) {
> *last = ed->ed_next;
> ed->ed_next = NULL;
> } else if (ohci->rh_state == OHCI_RH_RUNNING) {
> *last = ed->ed_next;
> ed->ed_next = NULL;
> ed_schedule(ohci, ed);
> }
>
> The *last = ed->ed_next will make ed_rm_list to point to ed->ed_next
> and previously added ed by usb_kill_urb will be left unreferenced by
> ed_rm_list. This causes usb_kill_urb() hang forever waiting for
> finish_unlink to remove added ed from ed_rm_list.
>
> The main reason for hang in this race condtion is addition and removal
> of ed from ed_rm_list in the beginning during usb_kill_urb and later
> last* is modified in finish_unlinks().
>
> As suggested by Alan Stern, the solution for proper handling of
> ohci->ed_rm_list is to remove ed from the ed_rm_list before finishing
> any URBs. Then at the end, we can add ed back to the list if necessary.
>
> This properly handle the updated ohci->ed_rm_list in usb_kill_urb().
>
> Fixes:977dcfdc6031("USB:OHCI:don't lose track of EDs when a controller dies")
> Acked-by: Alan Stern 
> CC: 
> Signed-off-by: Aman Deep 
> Signed-off-by: Jeffy Chen 
> ---
>
> Changes in v6:
> This is a resend of Aman Deep's v5 patch [0], which solved the hang we
> hit [1]. (Thanks Aman :)
>
> The v5 has some format issues, so i slightly adjust the commit message.
>
> [0] https://www.spinics.net/lists/linux-usb/msg129010.html
> [1] https://bugs.chromium.org/p/chromium/issues/detail?id=803749
>
>  drivers/usb/host/ohci-q.c | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
> index b2ec8c399363..4ccb85a67bb3 100644
> --- a/drivers/usb/host/ohci-q.c
> +++ b/drivers/usb/host/ohci-q.c
> @@ -1019,6 +1019,8 @@ static void finish_unlinks(struct ohci_hcd *ohci)
>  * have modified this list.  normally it's just prepending
>  * entries (which we'd ignore), but paranoia won't hurt.
>  */
> +   *last = ed->ed_next;
> +   ed->ed_next = NULL;
> modified = 0;
>
> /* unlink urbs as requested, but rescan the list after
> @@ -1077,21 +1079,22 @@ static void finish_unlinks(struct ohci_hcd *ohci)
> goto rescan_this;
>
> /*
> -* If no TDs are queued, take ED off the ed_rm_list.
> +* If no TDs are queued, ED is now idle.
>  * Otherwise, if the HC is running, reschedule.
> -* If not, leave it on the list for further dequeues.
> +* If the HC isn't running, add ED back to the
> +* start of the list for later processing.
>  */
> if (list_empty(>td_list)) {
> -   *last = ed->ed_next;
> -   ed->ed_next = NULL;
> ed->state = ED_IDLE;
> list_del(>in_use_list);
> } else if (ohci->rh_state == OHCI_RH_RUNNING) {
> -   *last = ed->ed_next;
> -   ed->ed_next = NULL;
> ed_schedule(ohci, ed);
> } else {
> -   last = >ed_next;
> +   ed->ed_next = ohci->ed_rm_list;
> +   ohci->ed_rm_list = ed;
> +   /* Don't loop on the same ED */
> +   if (last == >ed_rm_list)
> +   last = >ed_next;
> }
>
> if (modified)

I am experiencing a USB function call hang from userspace with OCHI
(full speed USB device) after updating from Linux 4.14.15 to 4.14.24
and noticed this commit.

Here is the Linux 4.14.24 kernel stack trace (extracted from SysRq+w
and amended with addr2line):
[] (__schedule) from [] (schedule+0x50/0xb4)
kernel/sched/core.c:2792
[] (schedule) from []
(usb_kill_urb.part.3+0x78/0xa8) include/asm-generic/preempt.h:59
[] (usb_kill_urb.part.3) from []
(usbdev_ioctl+0x1288/0x1cf0) drivers/usb/core/urb.c:690
[] (usbdev_ioctl) from []
(do_vfs_ioctl+0x9c/0x8ec) drivers/usb/core/devio.c:1835
[] (do_vfs_ioctl) from [] (SyS_ioctl+0x34/0x5c)
fs/ioctl.c:47
[] (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x54)
include/linux/file.h:39

Afterwards the 

RE: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX path

[Haiyang Zhang]

> -Original Message-
> From: Michael Kelley (EOSG)
> Sent: Saturday, March 24, 2018 12:48 PM
> To: Haiyang Zhang ; da...@davemloft.net;
> net...@vger.kernel.org
> Cc: KY Srinivasan ; Stephen Hemminger
> ; o...@aepfle.de; vkuzn...@redhat.com;
> de...@linuxdriverproject.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX path
> 
> > -Original Message-
> > From: linux-kernel-ow...@vger.kernel.org
> >  On Behalf Of Haiyang Zhang
> > Sent: Thursday, March 22, 2018 12:01 PM
> > To: da...@davemloft.net; net...@vger.kernel.org
> > Cc: Haiyang Zhang ; KY Srinivasan
> > ; Stephen Hemminger ;
> > o...@aepfle.de; vkuzn...@redhat.com; de...@linuxdriverproject.org;
> > linux-kernel@vger.kernel.org
> > Subject: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX
> > path
> >
> > From: Haiyang Zhang 
> >
> > As defined in hyperv_net.h, the NVSP_STAT_SUCCESS is one not zero.
> > Some functions returns 0 when it actually means NVSP_STAT_SUCCESS.
> > This patch fixes them.
> >
> > In netvsc_receive(), it puts the last RNDIS packet's receive status
> > for all packets in a vmxferpage which may contain multiple RNDIS
> > packets.
> > This patch puts NVSP_STAT_FAIL in the receive completion if one of the
> > packets in a vmxferpage fails.
> 
> This patch changes the status field that is being reported back to the Hyper-V
> host in the receive completion message in
> enq_receive_complete().   The current code reports 0 on success,
> and with the patch, it will report 1 on success.  So does this change affect
> anything on the Hyper-V side?  Or is Hyper-V just ignoring
> the value?   If this change doesn't have any impact on the
> interactions with Hyper-V, perhaps it would be good to explain why in the
> commit message.

Here is the definition of each status code for NetVSP. 
enum {
NVSP_STAT_NONE = 0,
NVSP_STAT_SUCCESS,
NVSP_STAT_FAIL,
NVSP_STAT_PROTOCOL_TOO_NEW,
NVSP_STAT_PROTOCOL_TOO_OLD,
NVSP_STAT_INVALID_RNDIS_PKT,
NVSP_STAT_BUSY,
NVSP_STAT_PROTOCOL_UNSUPPORTED,
NVSP_STAT_MAX,
};

Existing code returns NVSP_STAT_NONE = 0, and with this patch
we return NVSP_STAT_SUCCESS = 1. 
Based on testing, either way works for now. But for correctness
and future stability (e.g. host side becomes more stringent), we
should follow the protocol.

Thanks,
- Haiyang

RE: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX path

[Haiyang Zhang]

> -Original Message-
> From: Michael Kelley (EOSG)
> Sent: Saturday, March 24, 2018 12:48 PM
> To: Haiyang Zhang ; da...@davemloft.net;
> net...@vger.kernel.org
> Cc: KY Srinivasan ; Stephen Hemminger
> ; o...@aepfle.de; vkuzn...@redhat.com;
> de...@linuxdriverproject.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX path
> 
> > -Original Message-
> > From: linux-kernel-ow...@vger.kernel.org
> >  On Behalf Of Haiyang Zhang
> > Sent: Thursday, March 22, 2018 12:01 PM
> > To: da...@davemloft.net; net...@vger.kernel.org
> > Cc: Haiyang Zhang ; KY Srinivasan
> > ; Stephen Hemminger ;
> > o...@aepfle.de; vkuzn...@redhat.com; de...@linuxdriverproject.org;
> > linux-kernel@vger.kernel.org
> > Subject: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX
> > path
> >
> > From: Haiyang Zhang 
> >
> > As defined in hyperv_net.h, the NVSP_STAT_SUCCESS is one not zero.
> > Some functions returns 0 when it actually means NVSP_STAT_SUCCESS.
> > This patch fixes them.
> >
> > In netvsc_receive(), it puts the last RNDIS packet's receive status
> > for all packets in a vmxferpage which may contain multiple RNDIS
> > packets.
> > This patch puts NVSP_STAT_FAIL in the receive completion if one of the
> > packets in a vmxferpage fails.
> 
> This patch changes the status field that is being reported back to the Hyper-V
> host in the receive completion message in
> enq_receive_complete().   The current code reports 0 on success,
> and with the patch, it will report 1 on success.  So does this change affect
> anything on the Hyper-V side?  Or is Hyper-V just ignoring
> the value?   If this change doesn't have any impact on the
> interactions with Hyper-V, perhaps it would be good to explain why in the
> commit message.

Here is the definition of each status code for NetVSP. 
enum {
NVSP_STAT_NONE = 0,
NVSP_STAT_SUCCESS,
NVSP_STAT_FAIL,
NVSP_STAT_PROTOCOL_TOO_NEW,
NVSP_STAT_PROTOCOL_TOO_OLD,
NVSP_STAT_INVALID_RNDIS_PKT,
NVSP_STAT_BUSY,
NVSP_STAT_PROTOCOL_UNSUPPORTED,
NVSP_STAT_MAX,
};

Existing code returns NVSP_STAT_NONE = 0, and with this patch
we return NVSP_STAT_SUCCESS = 1. 
Based on testing, either way works for now. But for correctness
and future stability (e.g. host side becomes more stringent), we
should follow the protocol.

Thanks,
- Haiyang

Re: [PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

[Yang Shi]

On 3/23/18 9:30 PM, Matthew Wilcox wrote:

So, introduce a new rwlock in mm_struct to protect the concurrent access
to arg_start|end and env_start|end.

I don't think an rwlock makes much sense here.  There is almost no
concurrency on the read side, and an rwlock is more expensive than
a spinlock.  Just use a spinlock.


Thanks for the comment. Yes, actually there is not concurrency on the 
read side, will change to regular spin lock.


Yang

Re: [PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

[Yang Shi]

On 3/23/18 9:30 PM, Matthew Wilcox wrote:

So, introduce a new rwlock in mm_struct to protect the concurrent access
to arg_start|end and env_start|end.

I don't think an rwlock makes much sense here.  There is almost no
concurrency on the read side, and an rwlock is more expensive than
a spinlock.  Just use a spinlock.


Thanks for the comment. Yes, actually there is not concurrency on the 
read side, will change to regular spin lock.


Yang

RE: [PATCH v3] cpuidle: poll_state: Add time limit to poll_idle()

[Doug Smythies]

On 2018.03.14 07:04 Rafael J. Wysocki wrote:

> If poll_idle() is allowed to spin until need_resched() returns 'true',
> it may actually spin for a much longer time than expected by the idle
> governor, since set_tsk_need_resched() is not always called by the
> timer interrupt handler.  If that happens, the CPU may spend much
> more time than anticipated in the "polling" state.
>
> To prevent that from happening, limit the time of the spinning loop
> in poll_idle().

...[snip]...

> +#define POLL_IDLE_TIME_LIMIT (TICK_NSEC / 16)

The other ongoing threads on this aside, potentially, there might
be another issue.

What if the next available idle state, after 0, has a residency
that is greater than TICK_NSEC / 16? Meaning these numbers, for example:

/sys/devices/system/cpu/cpu0/cpuidle/state*/residency

The suggestion is that upon a timeout exit from idle state 0,
the measured_us should maybe be rejected, because the statistics
are being biased and it doesn't seem to correct itself.

Up to 1300% (<- not a typo) extra power consumption has been observed.

Supporting experimental data:

My processor: 
/sys/devices/system/cpu/cpu0/cpuidle/state0/residency:0
/sys/devices/system/cpu/cpu0/cpuidle/state1/residency:2
/sys/devices/system/cpu/cpu0/cpuidle/state2/residency:20
/sys/devices/system/cpu/cpu0/cpuidle/state3/residency:211 <<< Important
/sys/devices/system/cpu/cpu0/cpuidle/state4/residency:345

A 1000 Hz kernel (TICK_NSEC/16) = 62.5 nsec; idle system:

Idle state 0 time: Typically 0 uSec.
Processor package power: 3.7 watts (steady)

Now, disable idle states 1 and 2:

Idle state 0 time (all 8 CPUs): ~~ 430 Seconds / minute
Processor package power: ~52 watts (1300% more power, 14X)

A 250 Hz kernel (TICK_NSEC/16) = 250 nSec; idle system:

Idle state 0 time: Typically < 1 mSec / minute
Processor package power: 3.7 to 3.8 watts

Now, disable idle states 1 and 2:

Idle state 0 time (all 8 CPUs): Typically 0 to 70 mSecs / minute
Processor package power: 3.7 to 3.8 watts

A 1000 Hz kernel with:

+#define POLL_IDLE_TIME_LIMIT   (TICK_NSEC / 4)

Note: Just for a test. I am not suggesting this should change.

instead. i.e. (TICK_NSEC/4) = 250 nSec.

Idle state 0 time: Typically 0 uSec.
Processor package power: 3.7 watts (steady)

Now, disable idle states 1 and 2:

Idle state 0 time (all 8 CPUs): Typically 0 to 70 mSecs / minute
Processor package power: ~3.8 watts

Note 1: My example is contrived via disabling idle states, so
I don't know if it actually needs to be worried about.

Note 2: I do not know if there is some processor where
cpuidle/state1/residency is > 62.5 nSec.

Note 3: I am trying to figure out a way to test rejecting
measured_us upon timeout exit, but haven't made much progress.

... Doug

RE: [PATCH v3] cpuidle: poll_state: Add time limit to poll_idle()

[Doug Smythies]

On 2018.03.14 07:04 Rafael J. Wysocki wrote:

> If poll_idle() is allowed to spin until need_resched() returns 'true',
> it may actually spin for a much longer time than expected by the idle
> governor, since set_tsk_need_resched() is not always called by the
> timer interrupt handler.  If that happens, the CPU may spend much
> more time than anticipated in the "polling" state.
>
> To prevent that from happening, limit the time of the spinning loop
> in poll_idle().

...[snip]...

> +#define POLL_IDLE_TIME_LIMIT (TICK_NSEC / 16)

The other ongoing threads on this aside, potentially, there might
be another issue.

What if the next available idle state, after 0, has a residency
that is greater than TICK_NSEC / 16? Meaning these numbers, for example:

/sys/devices/system/cpu/cpu0/cpuidle/state*/residency

The suggestion is that upon a timeout exit from idle state 0,
the measured_us should maybe be rejected, because the statistics
are being biased and it doesn't seem to correct itself.

Up to 1300% (<- not a typo) extra power consumption has been observed.

Supporting experimental data:

My processor: 
/sys/devices/system/cpu/cpu0/cpuidle/state0/residency:0
/sys/devices/system/cpu/cpu0/cpuidle/state1/residency:2
/sys/devices/system/cpu/cpu0/cpuidle/state2/residency:20
/sys/devices/system/cpu/cpu0/cpuidle/state3/residency:211 <<< Important
/sys/devices/system/cpu/cpu0/cpuidle/state4/residency:345

A 1000 Hz kernel (TICK_NSEC/16) = 62.5 nsec; idle system:

Idle state 0 time: Typically 0 uSec.
Processor package power: 3.7 watts (steady)

Now, disable idle states 1 and 2:

Idle state 0 time (all 8 CPUs): ~~ 430 Seconds / minute
Processor package power: ~52 watts (1300% more power, 14X)

A 250 Hz kernel (TICK_NSEC/16) = 250 nSec; idle system:

Idle state 0 time: Typically < 1 mSec / minute
Processor package power: 3.7 to 3.8 watts

Now, disable idle states 1 and 2:

Idle state 0 time (all 8 CPUs): Typically 0 to 70 mSecs / minute
Processor package power: 3.7 to 3.8 watts

A 1000 Hz kernel with:

+#define POLL_IDLE_TIME_LIMIT   (TICK_NSEC / 4)

Note: Just for a test. I am not suggesting this should change.

instead. i.e. (TICK_NSEC/4) = 250 nSec.

Idle state 0 time: Typically 0 uSec.
Processor package power: 3.7 watts (steady)

Now, disable idle states 1 and 2:

Idle state 0 time (all 8 CPUs): Typically 0 to 70 mSecs / minute
Processor package power: ~3.8 watts

Note 1: My example is contrived via disabling idle states, so
I don't know if it actually needs to be worried about.

Note 2: I do not know if there is some processor where
cpuidle/state1/residency is > 62.5 nSec.

Note 3: I am trying to figure out a way to test rejecting
measured_us upon timeout exit, but haven't made much progress.

... Doug

Re: [PATCH net-next v2 2/2] cxgb4: collect hardware dump in second kernel

[Eric W. Biederman]

Thadeu Lima de Souza Cascardo  writes:

> On Sat, Mar 24, 2018 at 04:26:34PM +0530, Rahul Lakkireddy wrote:
>> Register callback to collect hardware/firmware dumps in second kernel
>> before hardware/firmware is initialized.  The dumps for each device
>> will be available under /sys/kernel/crashdd/cxgb4/ directory in second
>> kernel.
>> 
>> Signed-off-by: Rahul Lakkireddy 
>> Signed-off-by: Ganesh Goudar 
>> ---
>> v2:
>> - No Changes.
>> 
>> Changes since rfc v2:
>> - Update comments and commit message for sysfs change.
>> 
>> rfc v2:
>> - Updated dump registration to the new API in patch 1.
> [...]
>> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c 
>> b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
>> index e880be8e3c45..265cb026f868 100644
>> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
>> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
>> @@ -5527,6 +5527,18 @@ static int init_one(struct pci_dev *pdev, const 
>> struct pci_device_id *ent)
>>  if (err)
>>  goto out_free_adapter;
>>  
>> +if (is_kdump_kernel()) {
>> +/* Collect hardware state and append to
>> + * /sys/kernel/crashdd/cxgb4/ directory
>> + */
>> +err = cxgb4_cudbg_crashdd_add_dump(adapter);
>> +if (err) {
>> +dev_warn(adapter->pdev_dev,
>> + "Fail collecting crash device dump, err: %d. 
>> Continuing\n",
>> + err);
>> +err = 0;
>> +}
>> +}
>>  
>
> The problem I see with this approach is that you require that the driver
> is built into the kdump kernel (or present as a module in the kdump
> initramfs), and that you will probe the device during the collection of
> the dumps.

Compared to doing something in a crashing kernel anything in the kdump
kernel is a walk in the park.  Nothing is trustable in a crashing
kernel.

> IMHO, if you are going to require the device to be probed by the same
> driver during kdump, you might just as well use the device object itself
> to present the crash data. I think that's what Stephen Hemminger meant
> when he said to use sysfs. No need at all for any special crashdd. Just
> add an attribute or attribute group to the device object.

Doing something with the device model might make sense.  I am not
certain it does.  It is quite possible the device is in such a weird
state that the device driver fails to initialize.  That doesn't
mean the device driver can't scrape the registers and present
meaningful information to the rest of the system.

Whatever you do with capturing the state needs to happen early before
the driver initializes and stomps on the relevant state.

I don't expect there is much for the driver model to do, unless we wish
to do something explicitly before the normal device probe methods
happen.  What we need is the infrastructure for catching what gets
read from the driver and placing it in the core dump.

> Otherwise, as Eric Biederman pointed out, you should just add that data
> into the vmcore before you kexec, so you don't even need to look at a
> different file, and the driver does not even need to be present in the
> kdump kernel.

No.  I do mean before a kexec on panic happens.  Doing anything with
gathering this kind of information before kexec on panic is a very very
very very bad idea that will almost certainly make crash dumps less
reliable.  Don't even think about doing extra work on the crash dump
path.  Not ever.  No.  No.  No.  No.

The reason we use kexec on panic instead just creating a core dump
in the kernel is that many have tried and no one has gotten the kernel
to create crash dumps when things go wrong and it matters.  Meanwhile
kexec on panic works more often than not.

I mean that /proc/vmcore is a device that is used to gather up the bits
of the crashing kernel and to present it in a format that is easy to
read/save.  The tools read /proc/vmcore.

The driver or whatever is gathering this information absolutely needs to
be in the kdump kernel.

Eric

Re: [PATCH net-next v2 2/2] cxgb4: collect hardware dump in second kernel

[Eric W. Biederman]

Thadeu Lima de Souza Cascardo  writes:

> On Sat, Mar 24, 2018 at 04:26:34PM +0530, Rahul Lakkireddy wrote:
>> Register callback to collect hardware/firmware dumps in second kernel
>> before hardware/firmware is initialized.  The dumps for each device
>> will be available under /sys/kernel/crashdd/cxgb4/ directory in second
>> kernel.
>> 
>> Signed-off-by: Rahul Lakkireddy 
>> Signed-off-by: Ganesh Goudar 
>> ---
>> v2:
>> - No Changes.
>> 
>> Changes since rfc v2:
>> - Update comments and commit message for sysfs change.
>> 
>> rfc v2:
>> - Updated dump registration to the new API in patch 1.
> [...]
>> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c 
>> b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
>> index e880be8e3c45..265cb026f868 100644
>> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
>> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
>> @@ -5527,6 +5527,18 @@ static int init_one(struct pci_dev *pdev, const 
>> struct pci_device_id *ent)
>>  if (err)
>>  goto out_free_adapter;
>>  
>> +if (is_kdump_kernel()) {
>> +/* Collect hardware state and append to
>> + * /sys/kernel/crashdd/cxgb4/ directory
>> + */
>> +err = cxgb4_cudbg_crashdd_add_dump(adapter);
>> +if (err) {
>> +dev_warn(adapter->pdev_dev,
>> + "Fail collecting crash device dump, err: %d. 
>> Continuing\n",
>> + err);
>> +err = 0;
>> +}
>> +}
>>  
>
> The problem I see with this approach is that you require that the driver
> is built into the kdump kernel (or present as a module in the kdump
> initramfs), and that you will probe the device during the collection of
> the dumps.

Compared to doing something in a crashing kernel anything in the kdump
kernel is a walk in the park.  Nothing is trustable in a crashing
kernel.

> IMHO, if you are going to require the device to be probed by the same
> driver during kdump, you might just as well use the device object itself
> to present the crash data. I think that's what Stephen Hemminger meant
> when he said to use sysfs. No need at all for any special crashdd. Just
> add an attribute or attribute group to the device object.

Doing something with the device model might make sense.  I am not
certain it does.  It is quite possible the device is in such a weird
state that the device driver fails to initialize.  That doesn't
mean the device driver can't scrape the registers and present
meaningful information to the rest of the system.

Whatever you do with capturing the state needs to happen early before
the driver initializes and stomps on the relevant state.

I don't expect there is much for the driver model to do, unless we wish
to do something explicitly before the normal device probe methods
happen.  What we need is the infrastructure for catching what gets
read from the driver and placing it in the core dump.

> Otherwise, as Eric Biederman pointed out, you should just add that data
> into the vmcore before you kexec, so you don't even need to look at a
> different file, and the driver does not even need to be present in the
> kdump kernel.

No.  I do mean before a kexec on panic happens.  Doing anything with
gathering this kind of information before kexec on panic is a very very
very very bad idea that will almost certainly make crash dumps less
reliable.  Don't even think about doing extra work on the crash dump
path.  Not ever.  No.  No.  No.  No.

The reason we use kexec on panic instead just creating a core dump
in the kernel is that many have tried and no one has gotten the kernel
to create crash dumps when things go wrong and it matters.  Meanwhile
kexec on panic works more often than not.

I mean that /proc/vmcore is a device that is used to gather up the bits
of the crashing kernel and to present it in a format that is easy to
read/save.  The tools read /proc/vmcore.

The driver or whatever is gathering this information absolutely needs to
be in the kdump kernel.

Eric

Re: [RFC PATCH 5/6] sched/fair: Select an energy-efficient CPU on task wake-up

[Joel Fernandes]

On Fri, Mar 23, 2018 at 6:47 PM, Quentin Perret  wrote:
> On Thursday 22 Mar 2018 at 13:19:03 (-0700), Joel Fernandes wrote:
>> On Thu, Mar 22, 2018 at 11:06 AM, Patrick Bellasi
>>  wrote:
>
> [...]
>
>> >> > @@ -6555,6 +6613,14 @@ select_task_rq_fair(struct task_struct *p, int 
>> >> > prev_cpu, int sd_flag, int wake_f
>> >> > break;
>> >> > }
>> >> >
>> >> > +   /*
>> >> > +* Energy-aware task placement is performed on the 
>> >> > highest
>> >> > +* non-overutilized domain spanning over cpu and 
>> >> > prev_cpu.
>> >> > +*/
>> >> > +   if (want_energy && !sd_overutilized(tmp) &&
>> >> > +   cpumask_test_cpu(prev_cpu, sched_domain_span(tmp)))
>> >>
>> >> Shouldn't you check for the SD_ASYM_CPUCAPACITY flag here for tmp level?
>> >
>> > ... and this then should be covered by the previous check in
>> > wake_energy(), which sets want_energy.
>>
>> Right, but in a scenario which probably doesn't exist today where we
>> have both SD_ASYM_CPUCAPACITY and !SD_ASYM_CPUCAPACITY domains in the
>> hierarchy for which want_energy = 1, I was thinking if its more future
>> proof to check it and not make assumptions...
>
> So we can definitely have cases where SD_ASYM_CPUCAPACITY is not set at all
> sd levels. Today, on mobile systems, this flag is typically set only at DIE
> level for big.LITTLE platforms, and not at MC level.
> We enable EAS if we find _at least_ one domain that has this flag in the
> hierarchy, just to make sure we don't enable EAS for symmetric platform.
> It's just a way to check a property about the topology when EAS starts, not
> really a way to actually select the sd at which we do scheduling at
> runtime.

Yes Ok you're right we do have the ASYM flag set at some sd levels but
not others at the moment. Sorry about the hasty comment. I understand
what you're doing now, I am Ok with that.

thanks,

- Joel

Re: [RFC PATCH 5/6] sched/fair: Select an energy-efficient CPU on task wake-up

[Joel Fernandes]

On Fri, Mar 23, 2018 at 6:47 PM, Quentin Perret  wrote:
> On Thursday 22 Mar 2018 at 13:19:03 (-0700), Joel Fernandes wrote:
>> On Thu, Mar 22, 2018 at 11:06 AM, Patrick Bellasi
>>  wrote:
>
> [...]
>
>> >> > @@ -6555,6 +6613,14 @@ select_task_rq_fair(struct task_struct *p, int 
>> >> > prev_cpu, int sd_flag, int wake_f
>> >> > break;
>> >> > }
>> >> >
>> >> > +   /*
>> >> > +* Energy-aware task placement is performed on the 
>> >> > highest
>> >> > +* non-overutilized domain spanning over cpu and 
>> >> > prev_cpu.
>> >> > +*/
>> >> > +   if (want_energy && !sd_overutilized(tmp) &&
>> >> > +   cpumask_test_cpu(prev_cpu, sched_domain_span(tmp)))
>> >>
>> >> Shouldn't you check for the SD_ASYM_CPUCAPACITY flag here for tmp level?
>> >
>> > ... and this then should be covered by the previous check in
>> > wake_energy(), which sets want_energy.
>>
>> Right, but in a scenario which probably doesn't exist today where we
>> have both SD_ASYM_CPUCAPACITY and !SD_ASYM_CPUCAPACITY domains in the
>> hierarchy for which want_energy = 1, I was thinking if its more future
>> proof to check it and not make assumptions...
>
> So we can definitely have cases where SD_ASYM_CPUCAPACITY is not set at all
> sd levels. Today, on mobile systems, this flag is typically set only at DIE
> level for big.LITTLE platforms, and not at MC level.
> We enable EAS if we find _at least_ one domain that has this flag in the
> hierarchy, just to make sure we don't enable EAS for symmetric platform.
> It's just a way to check a property about the topology when EAS starts, not
> really a way to actually select the sd at which we do scheduling at
> runtime.

Yes Ok you're right we do have the ASYM flag set at some sd levels but
not others at the moment. Sorry about the hasty comment. I understand
what you're doing now, I am Ok with that.

thanks,

- Joel

Re: [REVIEW][PATCH 13/11] ipc/smack: Tidy up from the change in type of the ipc security hooks

[Casey Schaufler]

On 3/23/2018 10:42 PM, Eric W. Biederman wrote:
> Rename the variables shp, sma, msq to isp. As that is how the code already
> refers to those variables.

Thanks. It's important to keep the code readable.

> Collapse smack_of_shm, smack_of_sem, and smack_of_msq into smack_of_ipc,
> as the three functions had become completely identical.

Thanks. Completely reasonable and correct.

> Collapse smack_shm_alloc_security, smack_sem_alloc_security and
> smack_msg_queue_alloc_security into smack_ipc_alloc_security as the
> three functions had become identical.
>
> Collapse smack_shm_free_security, smack_sem_free_security and
> smack_msg_queue_free_security into smack_ipc_free_security as the three
> functions had become identical.

This is reasonable but unprecedented. Nowhere else is the
same function used to supply multiple LSM hooks. Does anyone
out there see a reason not to do this?

> Requested-by: Casey Schaufler 
> Signed-off-by: "Eric W. Biederman" 
> ---
>  security/smack/smack_lsm.c | 197 
> +
>  1 file changed, 58 insertions(+), 139 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index d960c2ea8d79..0735b8db158b 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2945,25 +2945,24 @@ static void smack_msg_msg_free_security(struct 
> msg_msg *msg)
>  }
>  
>  /**
> - * smack_of_shm - the smack pointer for the shm
> - * @shp: the object
> + * smack_of_ipc - the smack pointer for the ipc
> + * @isp: the object
>   *
>   * Returns a pointer to the smack value
>   */
> -static struct smack_known *smack_of_shm(struct kern_ipc_perm *shp)
> +static struct smack_known *smack_of_ipc(struct kern_ipc_perm *isp)
>  {
> - return (struct smack_known *)shp->security;
> + return (struct smack_known *)isp->security;
>  }
>  
>  /**
> - * smack_shm_alloc_security - Set the security blob for shm
> - * @shp: the object
> + * smack_ipc_alloc_security - Set the security blob for ipc
> + * @isp: the object
>   *
>   * Returns 0
>   */
> -static int smack_shm_alloc_security(struct kern_ipc_perm *shp)
> +static int smack_ipc_alloc_security(struct kern_ipc_perm *isp)
>  {
> - struct kern_ipc_perm *isp = shp;
>   struct smack_known *skp = smk_of_current();
>  
>   isp->security = skp;
> @@ -2971,34 +2970,32 @@ static int smack_shm_alloc_security(struct 
> kern_ipc_perm *shp)
>  }
>  
>  /**
> - * smack_shm_free_security - Clear the security blob for shm
> - * @shp: the object
> + * smack_ipc_free_security - Clear the security blob for ipc
> + * @isp: the object
>   *
>   * Clears the blob pointer
>   */
> -static void smack_shm_free_security(struct kern_ipc_perm *shp)
> +static void smack_ipc_free_security(struct kern_ipc_perm *isp)
>  {
> - struct kern_ipc_perm *isp = shp;
> -
>   isp->security = NULL;
>  }
>  
>  /**
>   * smk_curacc_shm : check if current has access on shm
> - * @shp : the object
> + * @isp : the object
>   * @access : access requested
>   *
>   * Returns 0 if current has the requested access, error code otherwise
>   */
> -static int smk_curacc_shm(struct kern_ipc_perm *shp, int access)
> +static int smk_curacc_shm(struct kern_ipc_perm *isp, int access)
>  {
> - struct smack_known *ssp = smack_of_shm(shp);
> + struct smack_known *ssp = smack_of_ipc(isp);
>   struct smk_audit_info ad;
>   int rc;
>  
>  #ifdef CONFIG_AUDIT
>   smk_ad_init(, __func__, LSM_AUDIT_DATA_IPC);
> - ad.a.u.ipc_id = shp->id;
> + ad.a.u.ipc_id = isp->id;
>  #endif
>   rc = smk_curacc(ssp, access, );
>   rc = smk_bu_current("shm", ssp, access, rc);
> @@ -3007,27 +3004,27 @@ static int smk_curacc_shm(struct kern_ipc_perm *shp, 
> int access)
>  
>  /**
>   * smack_shm_associate - Smack access check for shm
> - * @shp: the object
> + * @isp: the object
>   * @shmflg: access requested
>   *
>   * Returns 0 if current has the requested access, error code otherwise
>   */
> -static int smack_shm_associate(struct kern_ipc_perm *shp, int shmflg)
> +static int smack_shm_associate(struct kern_ipc_perm *isp, int shmflg)
>  {
>   int may;
>  
>   may = smack_flags_to_may(shmflg);
> - return smk_curacc_shm(shp, may);
> + return smk_curacc_shm(isp, may);
>  }
>  
>  /**
>   * smack_shm_shmctl - Smack access check for shm
> - * @shp: the object
> + * @isp: the object
>   * @cmd: what it wants to do
>   *
>   * Returns 0 if current has the requested access, error code otherwise
>   */
> -static int smack_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
> +static int smack_shm_shmctl(struct kern_ipc_perm *isp, int cmd)
>  {
>   int may;
>  
> @@ -3051,81 +3048,42 @@ static int smack_shm_shmctl(struct kern_ipc_perm 
> *shp, int cmd)
>   default:
>   return -EINVAL;
>   }
> - return smk_curacc_shm(shp, may);
> + return smk_curacc_shm(isp, may);
>  }
>  
>  /**
>   * smack_shm_shmat - 

Re: [REVIEW][PATCH 13/11] ipc/smack: Tidy up from the change in type of the ipc security hooks

[Casey Schaufler]

On 3/23/2018 10:42 PM, Eric W. Biederman wrote:
> Rename the variables shp, sma, msq to isp. As that is how the code already
> refers to those variables.

Thanks. It's important to keep the code readable.

> Collapse smack_of_shm, smack_of_sem, and smack_of_msq into smack_of_ipc,
> as the three functions had become completely identical.

Thanks. Completely reasonable and correct.

> Collapse smack_shm_alloc_security, smack_sem_alloc_security and
> smack_msg_queue_alloc_security into smack_ipc_alloc_security as the
> three functions had become identical.
>
> Collapse smack_shm_free_security, smack_sem_free_security and
> smack_msg_queue_free_security into smack_ipc_free_security as the three
> functions had become identical.

This is reasonable but unprecedented. Nowhere else is the
same function used to supply multiple LSM hooks. Does anyone
out there see a reason not to do this?

> Requested-by: Casey Schaufler 
> Signed-off-by: "Eric W. Biederman" 
> ---
>  security/smack/smack_lsm.c | 197 
> +
>  1 file changed, 58 insertions(+), 139 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index d960c2ea8d79..0735b8db158b 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2945,25 +2945,24 @@ static void smack_msg_msg_free_security(struct 
> msg_msg *msg)
>  }
>  
>  /**
> - * smack_of_shm - the smack pointer for the shm
> - * @shp: the object
> + * smack_of_ipc - the smack pointer for the ipc
> + * @isp: the object
>   *
>   * Returns a pointer to the smack value
>   */
> -static struct smack_known *smack_of_shm(struct kern_ipc_perm *shp)
> +static struct smack_known *smack_of_ipc(struct kern_ipc_perm *isp)
>  {
> - return (struct smack_known *)shp->security;
> + return (struct smack_known *)isp->security;
>  }
>  
>  /**
> - * smack_shm_alloc_security - Set the security blob for shm
> - * @shp: the object
> + * smack_ipc_alloc_security - Set the security blob for ipc
> + * @isp: the object
>   *
>   * Returns 0
>   */
> -static int smack_shm_alloc_security(struct kern_ipc_perm *shp)
> +static int smack_ipc_alloc_security(struct kern_ipc_perm *isp)
>  {
> - struct kern_ipc_perm *isp = shp;
>   struct smack_known *skp = smk_of_current();
>  
>   isp->security = skp;
> @@ -2971,34 +2970,32 @@ static int smack_shm_alloc_security(struct 
> kern_ipc_perm *shp)
>  }
>  
>  /**
> - * smack_shm_free_security - Clear the security blob for shm
> - * @shp: the object
> + * smack_ipc_free_security - Clear the security blob for ipc
> + * @isp: the object
>   *
>   * Clears the blob pointer
>   */
> -static void smack_shm_free_security(struct kern_ipc_perm *shp)
> +static void smack_ipc_free_security(struct kern_ipc_perm *isp)
>  {
> - struct kern_ipc_perm *isp = shp;
> -
>   isp->security = NULL;
>  }
>  
>  /**
>   * smk_curacc_shm : check if current has access on shm
> - * @shp : the object
> + * @isp : the object
>   * @access : access requested
>   *
>   * Returns 0 if current has the requested access, error code otherwise
>   */
> -static int smk_curacc_shm(struct kern_ipc_perm *shp, int access)
> +static int smk_curacc_shm(struct kern_ipc_perm *isp, int access)
>  {
> - struct smack_known *ssp = smack_of_shm(shp);
> + struct smack_known *ssp = smack_of_ipc(isp);
>   struct smk_audit_info ad;
>   int rc;
>  
>  #ifdef CONFIG_AUDIT
>   smk_ad_init(, __func__, LSM_AUDIT_DATA_IPC);
> - ad.a.u.ipc_id = shp->id;
> + ad.a.u.ipc_id = isp->id;
>  #endif
>   rc = smk_curacc(ssp, access, );
>   rc = smk_bu_current("shm", ssp, access, rc);
> @@ -3007,27 +3004,27 @@ static int smk_curacc_shm(struct kern_ipc_perm *shp, 
> int access)
>  
>  /**
>   * smack_shm_associate - Smack access check for shm
> - * @shp: the object
> + * @isp: the object
>   * @shmflg: access requested
>   *
>   * Returns 0 if current has the requested access, error code otherwise
>   */
> -static int smack_shm_associate(struct kern_ipc_perm *shp, int shmflg)
> +static int smack_shm_associate(struct kern_ipc_perm *isp, int shmflg)
>  {
>   int may;
>  
>   may = smack_flags_to_may(shmflg);
> - return smk_curacc_shm(shp, may);
> + return smk_curacc_shm(isp, may);
>  }
>  
>  /**
>   * smack_shm_shmctl - Smack access check for shm
> - * @shp: the object
> + * @isp: the object
>   * @cmd: what it wants to do
>   *
>   * Returns 0 if current has the requested access, error code otherwise
>   */
> -static int smack_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
> +static int smack_shm_shmctl(struct kern_ipc_perm *isp, int cmd)
>  {
>   int may;
>  
> @@ -3051,81 +3048,42 @@ static int smack_shm_shmctl(struct kern_ipc_perm 
> *shp, int cmd)
>   default:
>   return -EINVAL;
>   }
> - return smk_curacc_shm(shp, may);
> + return smk_curacc_shm(isp, may);
>  }
>  
>  /**
>   * smack_shm_shmat - Smack access for shmat
> - * @shp: the object
> + * 

[PATCH v2] gpio: ath79: Fix potential NULL dereference in ath79_gpio_probe()

[Alban Bedel]

From: Wei Yongjun 

platform_get_resource() may return NULL, add proper
check to avoid potential NULL dereferencing.

This is detected by Coccinelle semantic patch.

@@
expression pdev, res, n, t, e, e1, e2;
@@

res = platform_get_resource(pdev, t, n);
+ if (!res)
+   return -EINVAL;
... when != res == NULL
e = devm_ioremap(e1, res->start, e2);

Signed-off-by: Wei Yongjun 
[al...@free.fr: Fixed patch to apply on current tree]
Signed-off-by: Alban Bedel 
---
 drivers/gpio/gpio-ath79.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-ath79.c b/drivers/gpio/gpio-ath79.c
index 3ae7c18..684e9d6 100644
--- a/drivers/gpio/gpio-ath79.c
+++ b/drivers/gpio/gpio-ath79.c
@@ -258,6 +258,8 @@ static int ath79_gpio_probe(struct platform_device *pdev)
}
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   if (!res)
+   return -EINVAL;
ctrl->base = devm_ioremap_nocache(
>dev, res->start, resource_size(res));
if (!ctrl->base)
-- 
2.7.4

[PATCH v2] gpio: ath79: Fix potential NULL dereference in ath79_gpio_probe()

[Alban Bedel]

From: Wei Yongjun 

platform_get_resource() may return NULL, add proper
check to avoid potential NULL dereferencing.

This is detected by Coccinelle semantic patch.

@@
expression pdev, res, n, t, e, e1, e2;
@@

res = platform_get_resource(pdev, t, n);
+ if (!res)
+   return -EINVAL;
... when != res == NULL
e = devm_ioremap(e1, res->start, e2);

Signed-off-by: Wei Yongjun 
[al...@free.fr: Fixed patch to apply on current tree]
Signed-off-by: Alban Bedel 
---
 drivers/gpio/gpio-ath79.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-ath79.c b/drivers/gpio/gpio-ath79.c
index 3ae7c18..684e9d6 100644
--- a/drivers/gpio/gpio-ath79.c
+++ b/drivers/gpio/gpio-ath79.c
@@ -258,6 +258,8 @@ static int ath79_gpio_probe(struct platform_device *pdev)
}
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   if (!res)
+   return -EINVAL;
ctrl->base = devm_ioremap_nocache(
>dev, res->start, resource_size(res));
if (!ctrl->base)
-- 
2.7.4

[PATCH v3 3/3] mtd: Add support for reading MTD devices via the nvmem API

[Alban Bedel]

Allow drivers that use the nvmem API to read data stored on MTD devices.
For this the mtd devices are registered as read-only NVMEM providers.
On OF systems only devices that have the 'nvmem-provider' property
are registered, on non-OF system all MTD devices are registered.

Signed-off-by: Alban Bedel 
---
Changelog:
v2: * Moved to the MTD core instead of using notifiers
* Fixed the Kconfig description
v3: * Rebased on current kernel
* Moved the code to mtdcore.c and removed the conditional
  compilation as suggested by Boris Brezillon
* Fixed my name in From and Signed-off-by
* Only allow root to read from the nvmem sysfs interface
---
 drivers/mtd/Kconfig |  1 +
 drivers/mtd/mtdcore.c   | 59 +
 include/linux/mtd/mtd.h |  2 ++
 3 files changed, 62 insertions(+)

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 2a8ac68..911d869 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -1,5 +1,6 @@
 menuconfig MTD
tristate "Memory Technology Device (MTD) support"
+   imply NVMEM
help
  Memory Technology Devices are flash, RAM and similar chips, often
  used for solid state file systems on embedded devices. This option
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 28553c8..d2a127c 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -478,6 +478,48 @@ int mtd_pairing_groups(struct mtd_info *mtd)
 }
 EXPORT_SYMBOL_GPL(mtd_pairing_groups);
 
+static int mtd_nvmem_reg_read(void *priv, unsigned int offset,
+ void *val, size_t bytes)
+{
+   struct mtd_info *mtd = priv;
+   size_t retlen;
+   int err;
+
+   err = mtd_read(mtd, offset, bytes, , val);
+   if (err && err != -EUCLEAN)
+   return err;
+
+   return retlen == bytes ? 0 : -EIO;
+}
+
+static int mtd_nvmem_add(struct mtd_info *mtd)
+{
+   struct nvmem_config config = {};
+
+   config.dev = >dev;
+   config.owner = THIS_MODULE;
+   config.reg_read = mtd_nvmem_reg_read;
+   config.size = mtd->size;
+   config.word_size = 1;
+   config.stride = 1;
+   config.read_only = true;
+   config.root_only = true;
+   config.priv = mtd;
+
+   mtd->nvmem = nvmem_register();
+   if (IS_ERR(mtd->nvmem)) {
+   /* Just ignore if there is no NVMEM support in the kernel */
+   if (PTR_ERR(mtd->nvmem) == -ENOSYS) {
+   mtd->nvmem = NULL;
+   } else {
+   dev_err(>dev, "Failed to register NVMEM device\n");
+   return PTR_ERR(mtd->nvmem);
+   }
+   }
+
+   return 0;
+}
+
 static struct dentry *dfs_dir_mtd;
 
 /**
@@ -560,6 +602,11 @@ int add_mtd_device(struct mtd_info *mtd)
if (error)
goto fail_added;
 
+   /* Add the nvmem provider */
+   error = mtd_nvmem_add(mtd);
+   if (error)
+   goto fail_nvmem_add;
+
if (!IS_ERR_OR_NULL(dfs_dir_mtd)) {
mtd->dbg.dfs_dir = debugfs_create_dir(dev_name(>dev), 
dfs_dir_mtd);
if (IS_ERR_OR_NULL(mtd->dbg.dfs_dir)) {
@@ -585,6 +632,8 @@ int add_mtd_device(struct mtd_info *mtd)
__module_get(THIS_MODULE);
return 0;
 
+fail_nvmem_add:
+   device_unregister(>dev);
 fail_added:
of_node_put(mtd_get_of_node(mtd));
idr_remove(_idr, i);
@@ -627,6 +676,16 @@ int del_mtd_device(struct mtd_info *mtd)
   mtd->index, mtd->name, mtd->usecount);
ret = -EBUSY;
} else {
+   /* Try to remove the NVMEM provider */
+   if (mtd->nvmem) {
+   ret = nvmem_unregister(mtd->nvmem);
+   if (ret) {
+   dev_err(>dev,
+   "Failed to unregister NVMEM device\n");
+   goto out_error;
+   }
+   }
+
device_unregister(>dev);
 
idr_remove(_idr, mtd->index);
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 205eded..660b8e7 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -352,6 +353,7 @@ struct mtd_info {
struct device dev;
int usecount;
struct mtd_debug_info dbg;
+   struct nvmem_device *nvmem;
 };
 
 int mtd_ooblayout_ecc(struct mtd_info *mtd, int section,
-- 
2.7.4

[PATCH v3 2/3] doc: bindings: Add bindings documentation for mtd nvmem

[Alban Bedel]

Config data for drivers, like MAC addresses, is often stored in MTD.
Add a binding that define how such data storage can be represented in
device tree.

Signed-off-by: Alban Bedel 
---
Changelog:
v2: * Added a "Required properties" section with the nvmem-provider
  property
v3: * Fixed my name in From and Signed-off-by
* Moved to the new nvmem binding with the nvmem-cells subnode
---
 .../devicetree/bindings/nvmem/mtd-nvmem.txt| 27 ++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt

diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt 
b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt
new file mode 100644
index 000..c819a69
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt
@@ -0,0 +1,27 @@
+= NVMEM in MTD =
+
+Config data for drivers, like MAC addresses, is often stored in MTD.
+An MTD device, or one of its partition, can be defined as a NVMEM provider
+by having an 'nvmem-cells' subnode as defined in nvmem.txt.
+
+Example:
+
+   flash@0 {
+   ...
+
+   partition@2 {
+   label = "art";
+   reg = <0x7F 0x01>;
+   read-only;
+
+   nvmem-cells {
+   compatible = "nvmem-cells";
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   eeprom@1000 {
+   reg = <0x1000 0x1000>;
+   };
+   };
+   };
+   };
-- 
2.7.4

[PATCH v3 3/3] mtd: Add support for reading MTD devices via the nvmem API

[Alban Bedel]

Allow drivers that use the nvmem API to read data stored on MTD devices.
For this the mtd devices are registered as read-only NVMEM providers.
On OF systems only devices that have the 'nvmem-provider' property
are registered, on non-OF system all MTD devices are registered.

Signed-off-by: Alban Bedel 
---
Changelog:
v2: * Moved to the MTD core instead of using notifiers
* Fixed the Kconfig description
v3: * Rebased on current kernel
* Moved the code to mtdcore.c and removed the conditional
  compilation as suggested by Boris Brezillon
* Fixed my name in From and Signed-off-by
* Only allow root to read from the nvmem sysfs interface
---
 drivers/mtd/Kconfig |  1 +
 drivers/mtd/mtdcore.c   | 59 +
 include/linux/mtd/mtd.h |  2 ++
 3 files changed, 62 insertions(+)

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 2a8ac68..911d869 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -1,5 +1,6 @@
 menuconfig MTD
tristate "Memory Technology Device (MTD) support"
+   imply NVMEM
help
  Memory Technology Devices are flash, RAM and similar chips, often
  used for solid state file systems on embedded devices. This option
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 28553c8..d2a127c 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -478,6 +478,48 @@ int mtd_pairing_groups(struct mtd_info *mtd)
 }
 EXPORT_SYMBOL_GPL(mtd_pairing_groups);
 
+static int mtd_nvmem_reg_read(void *priv, unsigned int offset,
+ void *val, size_t bytes)
+{
+   struct mtd_info *mtd = priv;
+   size_t retlen;
+   int err;
+
+   err = mtd_read(mtd, offset, bytes, , val);
+   if (err && err != -EUCLEAN)
+   return err;
+
+   return retlen == bytes ? 0 : -EIO;
+}
+
+static int mtd_nvmem_add(struct mtd_info *mtd)
+{
+   struct nvmem_config config = {};
+
+   config.dev = >dev;
+   config.owner = THIS_MODULE;
+   config.reg_read = mtd_nvmem_reg_read;
+   config.size = mtd->size;
+   config.word_size = 1;
+   config.stride = 1;
+   config.read_only = true;
+   config.root_only = true;
+   config.priv = mtd;
+
+   mtd->nvmem = nvmem_register();
+   if (IS_ERR(mtd->nvmem)) {
+   /* Just ignore if there is no NVMEM support in the kernel */
+   if (PTR_ERR(mtd->nvmem) == -ENOSYS) {
+   mtd->nvmem = NULL;
+   } else {
+   dev_err(>dev, "Failed to register NVMEM device\n");
+   return PTR_ERR(mtd->nvmem);
+   }
+   }
+
+   return 0;
+}
+
 static struct dentry *dfs_dir_mtd;
 
 /**
@@ -560,6 +602,11 @@ int add_mtd_device(struct mtd_info *mtd)
if (error)
goto fail_added;
 
+   /* Add the nvmem provider */
+   error = mtd_nvmem_add(mtd);
+   if (error)
+   goto fail_nvmem_add;
+
if (!IS_ERR_OR_NULL(dfs_dir_mtd)) {
mtd->dbg.dfs_dir = debugfs_create_dir(dev_name(>dev), 
dfs_dir_mtd);
if (IS_ERR_OR_NULL(mtd->dbg.dfs_dir)) {
@@ -585,6 +632,8 @@ int add_mtd_device(struct mtd_info *mtd)
__module_get(THIS_MODULE);
return 0;
 
+fail_nvmem_add:
+   device_unregister(>dev);
 fail_added:
of_node_put(mtd_get_of_node(mtd));
idr_remove(_idr, i);
@@ -627,6 +676,16 @@ int del_mtd_device(struct mtd_info *mtd)
   mtd->index, mtd->name, mtd->usecount);
ret = -EBUSY;
} else {
+   /* Try to remove the NVMEM provider */
+   if (mtd->nvmem) {
+   ret = nvmem_unregister(mtd->nvmem);
+   if (ret) {
+   dev_err(>dev,
+   "Failed to unregister NVMEM device\n");
+   goto out_error;
+   }
+   }
+
device_unregister(>dev);
 
idr_remove(_idr, mtd->index);
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 205eded..660b8e7 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -352,6 +353,7 @@ struct mtd_info {
struct device dev;
int usecount;
struct mtd_debug_info dbg;
+   struct nvmem_device *nvmem;
 };
 
 int mtd_ooblayout_ecc(struct mtd_info *mtd, int section,
-- 
2.7.4

[PATCH v3 2/3] doc: bindings: Add bindings documentation for mtd nvmem

[Alban Bedel]

Config data for drivers, like MAC addresses, is often stored in MTD.
Add a binding that define how such data storage can be represented in
device tree.

Signed-off-by: Alban Bedel 
---
Changelog:
v2: * Added a "Required properties" section with the nvmem-provider
  property
v3: * Fixed my name in From and Signed-off-by
* Moved to the new nvmem binding with the nvmem-cells subnode
---
 .../devicetree/bindings/nvmem/mtd-nvmem.txt| 27 ++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt

diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt 
b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt
new file mode 100644
index 000..c819a69
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt
@@ -0,0 +1,27 @@
+= NVMEM in MTD =
+
+Config data for drivers, like MAC addresses, is often stored in MTD.
+An MTD device, or one of its partition, can be defined as a NVMEM provider
+by having an 'nvmem-cells' subnode as defined in nvmem.txt.
+
+Example:
+
+   flash@0 {
+   ...
+
+   partition@2 {
+   label = "art";
+   reg = <0x7F 0x01>;
+   read-only;
+
+   nvmem-cells {
+   compatible = "nvmem-cells";
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   eeprom@1000 {
+   reg = <0x1000 0x1000>;
+   };
+   };
+   };
+   };
-- 
2.7.4

[PATCH v3 1/3] nvmem: Update the OF binding to use a subnode for the cells list

[Alban Bedel]

Having the cells as subnodes of the provider device without any
compatible property might clash with other bindings. To avoid this
problem update the binding to have all the cells in a 'nvmem-cells'
subnode with a 'nvmem-cells' compatible string. This new binding
guarantee that we can turn any kind of device in a nvmem provider.

While discouraged for new uses the old scheme is still supported for
backward compatibility.

Signed-off-by: Alban Bedel 
---
 Documentation/devicetree/bindings/nvmem/nvmem.txt | 55 ---
 drivers/nvmem/core.c  | 10 +
 2 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.txt 
b/Documentation/devicetree/bindings/nvmem/nvmem.txt
index fd06c09..6b723e7 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.txt
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.txt
@@ -11,14 +11,29 @@ these data from, and where they are stored on the storage 
device.
 This document is here to document this.
 
 = Data providers =
-Contains bindings specific to provider drivers and data cells as children
-of this node.
+A data provider should have a subnode named 'nvmem-cells' that contains
+a subnodes for each data cells.
+
+For backward compatibility the nvmem data cells can be direct children
+of the data provider. This use is discouraged as it can conflict with
+other bindings.
 
 Optional properties:
  read-only: Mark the provider as read only.
 
+= Data cells list =
+The data cells list node should be named 'nvmem-cells' and have a
+child node for each data cell.
+
+Required properties:
+ compatible: Must be "nvmem-cells"
+ #address-cells: <1> if the provider use 32 bit addressing,
+ <2> for 64 bits addressing
+ #size-cells: <1> if the provider use 32 bit sizes,
+  <2> for 64 bits sizes
+
 = Data cells =
-These are the child nodes of the provider which contain data cell
+These are the child nodes of the nvmem-cells node which contain data cell
 information like offset and size in nvmem provider.
 
 Required properties:
@@ -37,24 +52,30 @@ For example:
...
 
/* Data cells */
-   tsens_calibration: calib@404 {
-   reg = <0x404 0x10>;
-   };
+   nvmem-cells {
+   compatible = "nvmem-cells";
+   #address-cells = <1>;
+   #size-cells = <1>;
 
-   tsens_calibration_bckp: calib_bckp@504 {
-   reg = <0x504 0x11>;
-   bits = <6 128>
-   };
+   tsens_calibration: calib@404 {
+   reg = <0x404 0x10>;
+   };
 
-   pvs_version: pvs-version@6 {
-   reg = <0x6 0x2>
-   bits = <7 2>
-   };
+   tsens_calibration_bckp: calib_bckp@504 {
+   reg = <0x504 0x11>;
+   bits = <6 128>
+   };
 
-   speed_bin: speed-bin@c{
-   reg = <0xc 0x1>;
-   bits = <2 3>;
+   pvs_version: pvs-version@6 {
+   reg = <0x6 0x2>
+   bits = <7 2>
+   };
 
+   speed_bin: speed-bin@c{
+   reg = <0xc 0x1>;
+   bits = <2 3>;
+
+   };
};
...
};
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 78051f0..a59195c 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -783,6 +783,16 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node 
*np,
if (!nvmem_np)
return ERR_PTR(-EINVAL);
 
+   /* Devices using the new binding have all the cells in
+* a subnode with compatible = "nvmem-cells". In this
+* case the device will be the parent of this node.
+*/
+   if (of_device_is_compatible(nvmem_np, "nvmem-cells")) {
+   nvmem_np = of_get_next_parent(nvmem_np);
+   if (!nvmem_np)
+   return ERR_PTR(-EINVAL);
+   }
+
nvmem = __nvmem_device_get(nvmem_np, NULL, NULL);
of_node_put(nvmem_np);
if (IS_ERR(nvmem))
-- 
2.7.4

[PATCH v3 0/3] mtd: Add support for reading MTD devices via the nvmem API

[Alban Bedel]

Hi all,

this series add support for reading MTD devices via the nvmem API, this
is mostly needed on embedded devices where things like MAC address and
calibration data is often stored in a partition on the main flash device.

Adding support for the nvmem API to the MTD core is trivial, however there
is a clash in the OF binding used by both subsystems. The current nvmem
binding expect nvmem cell to be subnode of the nvmem device, without any
compatible string. But MTD devices used a similar scheme for partition in
the past, so a subnode from an MTD device could be a partition using the
old binding or an nvmem cell.

To avoid this problem we update the nvmem cell binding to use a 'nvmem-cells'
subnode with compatible string to hold the list of nvmem cells. This new
binding make sure that any kind of device can be used as nvmem provider.

Alban Bedel (3):
  nvmem: Update the OF binding to use a subnode for the cells list
  doc: bindings: Add bindings documentation for mtd nvmem
  mtd: Add support for reading MTD devices via the nvmem API

 .../devicetree/bindings/nvmem/mtd-nvmem.txt| 27 ++
 Documentation/devicetree/bindings/nvmem/nvmem.txt  | 55 +---
 drivers/mtd/Kconfig|  1 +
 drivers/mtd/mtdcore.c  | 59 ++
 drivers/nvmem/core.c   | 10 
 include/linux/mtd/mtd.h|  2 +
 6 files changed, 137 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt

-- 
2.7.4

[PATCH v3 1/3] nvmem: Update the OF binding to use a subnode for the cells list

[Alban Bedel]

Having the cells as subnodes of the provider device without any
compatible property might clash with other bindings. To avoid this
problem update the binding to have all the cells in a 'nvmem-cells'
subnode with a 'nvmem-cells' compatible string. This new binding
guarantee that we can turn any kind of device in a nvmem provider.

While discouraged for new uses the old scheme is still supported for
backward compatibility.

Signed-off-by: Alban Bedel 
---
 Documentation/devicetree/bindings/nvmem/nvmem.txt | 55 ---
 drivers/nvmem/core.c  | 10 +
 2 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.txt 
b/Documentation/devicetree/bindings/nvmem/nvmem.txt
index fd06c09..6b723e7 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.txt
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.txt
@@ -11,14 +11,29 @@ these data from, and where they are stored on the storage 
device.
 This document is here to document this.
 
 = Data providers =
-Contains bindings specific to provider drivers and data cells as children
-of this node.
+A data provider should have a subnode named 'nvmem-cells' that contains
+a subnodes for each data cells.
+
+For backward compatibility the nvmem data cells can be direct children
+of the data provider. This use is discouraged as it can conflict with
+other bindings.
 
 Optional properties:
  read-only: Mark the provider as read only.
 
+= Data cells list =
+The data cells list node should be named 'nvmem-cells' and have a
+child node for each data cell.
+
+Required properties:
+ compatible: Must be "nvmem-cells"
+ #address-cells: <1> if the provider use 32 bit addressing,
+ <2> for 64 bits addressing
+ #size-cells: <1> if the provider use 32 bit sizes,
+  <2> for 64 bits sizes
+
 = Data cells =
-These are the child nodes of the provider which contain data cell
+These are the child nodes of the nvmem-cells node which contain data cell
 information like offset and size in nvmem provider.
 
 Required properties:
@@ -37,24 +52,30 @@ For example:
...
 
/* Data cells */
-   tsens_calibration: calib@404 {
-   reg = <0x404 0x10>;
-   };
+   nvmem-cells {
+   compatible = "nvmem-cells";
+   #address-cells = <1>;
+   #size-cells = <1>;
 
-   tsens_calibration_bckp: calib_bckp@504 {
-   reg = <0x504 0x11>;
-   bits = <6 128>
-   };
+   tsens_calibration: calib@404 {
+   reg = <0x404 0x10>;
+   };
 
-   pvs_version: pvs-version@6 {
-   reg = <0x6 0x2>
-   bits = <7 2>
-   };
+   tsens_calibration_bckp: calib_bckp@504 {
+   reg = <0x504 0x11>;
+   bits = <6 128>
+   };
 
-   speed_bin: speed-bin@c{
-   reg = <0xc 0x1>;
-   bits = <2 3>;
+   pvs_version: pvs-version@6 {
+   reg = <0x6 0x2>
+   bits = <7 2>
+   };
 
+   speed_bin: speed-bin@c{
+   reg = <0xc 0x1>;
+   bits = <2 3>;
+
+   };
};
...
};
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 78051f0..a59195c 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -783,6 +783,16 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node 
*np,
if (!nvmem_np)
return ERR_PTR(-EINVAL);
 
+   /* Devices using the new binding have all the cells in
+* a subnode with compatible = "nvmem-cells". In this
+* case the device will be the parent of this node.
+*/
+   if (of_device_is_compatible(nvmem_np, "nvmem-cells")) {
+   nvmem_np = of_get_next_parent(nvmem_np);
+   if (!nvmem_np)
+   return ERR_PTR(-EINVAL);
+   }
+
nvmem = __nvmem_device_get(nvmem_np, NULL, NULL);
of_node_put(nvmem_np);
if (IS_ERR(nvmem))
-- 
2.7.4

[PATCH v3 0/3] mtd: Add support for reading MTD devices via the nvmem API

[Alban Bedel]

Hi all,

this series add support for reading MTD devices via the nvmem API, this
is mostly needed on embedded devices where things like MAC address and
calibration data is often stored in a partition on the main flash device.

Adding support for the nvmem API to the MTD core is trivial, however there
is a clash in the OF binding used by both subsystems. The current nvmem
binding expect nvmem cell to be subnode of the nvmem device, without any
compatible string. But MTD devices used a similar scheme for partition in
the past, so a subnode from an MTD device could be a partition using the
old binding or an nvmem cell.

To avoid this problem we update the nvmem cell binding to use a 'nvmem-cells'
subnode with compatible string to hold the list of nvmem cells. This new
binding make sure that any kind of device can be used as nvmem provider.

Alban Bedel (3):
  nvmem: Update the OF binding to use a subnode for the cells list
  doc: bindings: Add bindings documentation for mtd nvmem
  mtd: Add support for reading MTD devices via the nvmem API

 .../devicetree/bindings/nvmem/mtd-nvmem.txt| 27 ++
 Documentation/devicetree/bindings/nvmem/nvmem.txt  | 55 +---
 drivers/mtd/Kconfig|  1 +
 drivers/mtd/mtdcore.c  | 59 ++
 drivers/nvmem/core.c   | 10 
 include/linux/mtd/mtd.h|  2 +
 6 files changed, 137 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt

-- 
2.7.4

Re: [PATCH v5 2/2] i2c: add support for Socionext SynQuacer I2C controller

[Wolfram Sang]

My code checkers mention this:

CPPCHECK
drivers/i2c/busses/i2c-synquacer.c:422: style: Checking if unsigned variable 
'timeout' is less than zero.


  CC  drivers/i2c/busses/i2c-synquacer.o
drivers/i2c/busses/i2c-synquacer.c: In function ‘synquacer_i2c_probe’:
drivers/i2c/busses/i2c-synquacer.c:678:15: warning: comparison of unsigned 
expression < 0 is always false [-Wtype-limits]
  if (i2c->irq < 0) {
   ^


signature.asc
Description: PGP signature

Re: [PATCH v5 2/2] i2c: add support for Socionext SynQuacer I2C controller

[Wolfram Sang]

My code checkers mention this:

CPPCHECK
drivers/i2c/busses/i2c-synquacer.c:422: style: Checking if unsigned variable 
'timeout' is less than zero.


  CC  drivers/i2c/busses/i2c-synquacer.o
drivers/i2c/busses/i2c-synquacer.c: In function ‘synquacer_i2c_probe’:
drivers/i2c/busses/i2c-synquacer.c:678:15: warning: comparison of unsigned 
expression < 0 is always false [-Wtype-limits]
  if (i2c->irq < 0) {
   ^


signature.asc
Description: PGP signature

[RFC PATCH] iommu/vt-d: intel_iommu_page_response() can be static

[kbuild test robot]

Fixes: 49c5977b4ee7 ("iommu/vt-d: add intel iommu page response function")
Signed-off-by: Fengguang Wu 
---
 intel-iommu.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 8d73ff0..06be796 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5195,7 +5195,7 @@ static int intel_iommu_sva_invalidate(struct iommu_domain 
*domain,
return ret;
 }
 
-int intel_iommu_page_response(struct device *dev, struct page_response_msg 
*msg)
+static int intel_iommu_page_response(struct device *dev, struct 
page_response_msg *msg)
 {
struct qi_desc resp;
struct intel_iommu *iommu;

[RFC PATCH] iommu/vt-d: intel_iommu_page_response() can be static

[kbuild test robot]

Fixes: 49c5977b4ee7 ("iommu/vt-d: add intel iommu page response function")
Signed-off-by: Fengguang Wu 
---
 intel-iommu.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 8d73ff0..06be796 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5195,7 +5195,7 @@ static int intel_iommu_sva_invalidate(struct iommu_domain 
*domain,
return ret;
 }
 
-int intel_iommu_page_response(struct device *dev, struct page_response_msg 
*msg)
+static int intel_iommu_page_response(struct device *dev, struct 
page_response_msg *msg)
 {
struct qi_desc resp;
struct intel_iommu *iommu;

Re: [PATCH v4 20/22] iommu/vt-d: add intel iommu page response function

[kbuild test robot]

Hi Jacob,

I love your patch! Perhaps something to improve:

[auto build test WARNING on iommu/next]
[also build test WARNING on v4.16-rc6 next-20180323]
[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/Jacob-Pan/IOMMU-and-VT-d-driver-support-for-Shared-Virtual-Address-SVA/20180325-024555
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   drivers/iommu/intel-iommu.c:500:5: sparse: symbol 'intel_iommu_gfx_mapped' 
was not declared. Should it be static?
   drivers/iommu/intel-iommu.c:3066:41: sparse: incorrect type in argument 1 
(different address spaces) @@expected void volatile [noderef] *addr 
@@got ile [noderef] *addr @@
   drivers/iommu/intel-iommu.c:3066:41:expected void volatile [noderef] 
*addr
   drivers/iommu/intel-iommu.c:3066:41:got struct context_entry *old_ce
>> drivers/iommu/intel-iommu.c:5198:5: sparse: symbol 
>> 'intel_iommu_page_response' was not declared. Should it be static?
>> drivers/iommu/intel-iommu.c:5224:48: sparse: right shift by bigger than 
>> source value

Please review and possibly fold the followup patch.

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

Re: [PATCH v4 20/22] iommu/vt-d: add intel iommu page response function

[kbuild test robot]

Hi Jacob,

I love your patch! Perhaps something to improve:

[auto build test WARNING on iommu/next]
[also build test WARNING on v4.16-rc6 next-20180323]
[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/Jacob-Pan/IOMMU-and-VT-d-driver-support-for-Shared-Virtual-Address-SVA/20180325-024555
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   drivers/iommu/intel-iommu.c:500:5: sparse: symbol 'intel_iommu_gfx_mapped' 
was not declared. Should it be static?
   drivers/iommu/intel-iommu.c:3066:41: sparse: incorrect type in argument 1 
(different address spaces) @@expected void volatile [noderef] *addr 
@@got ile [noderef] *addr @@
   drivers/iommu/intel-iommu.c:3066:41:expected void volatile [noderef] 
*addr
   drivers/iommu/intel-iommu.c:3066:41:got struct context_entry *old_ce
>> drivers/iommu/intel-iommu.c:5198:5: sparse: symbol 
>> 'intel_iommu_page_response' was not declared. Should it be static?
>> drivers/iommu/intel-iommu.c:5224:48: sparse: right shift by bigger than 
>> source value

Please review and possibly fold the followup patch.

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

Re: [PATCH v5 2/2] i2c: add support for Socionext SynQuacer I2C controller

[Wolfram Sang]

Hi Ard,

> +static int synquacer_i2c_master_start(struct synquacer_i2c *i2c,
> +   struct i2c_msg *pmsg)
> +{
> + unsigned char bsr, bcr;
> +
> + if (pmsg->flags & I2C_M_RD)
> + writeb((pmsg->addr << 1) | 1,
> +i2c->base + SYNQUACER_I2C_REG_DAR);
> + else
> + writeb(pmsg->addr << 1,
> +i2c->base + SYNQUACER_I2C_REG_DAR);

writeb(i2c_8bit_addr_from_msg(pmsg), i2c->base + SYNQUACER_I2C_REG_DAR);

?

> +static int synquacer_i2c_master_recover(struct synquacer_i2c *i2c)
> +{

This is the bus recovery mechanism with toggling SCL pulses, right?
That should be implemented using a 'struct i2c_bus_recovery_info' and
the core helpers.

> +static int synquacer_i2c_doxfer(struct synquacer_i2c *i2c,
> + struct i2c_msg *msgs, int num)
> +{
> + unsigned char bsr;
> + unsigned long timeout, bb_timeout;
> + int ret;
> +
> + if (i2c->is_suspended)
> + return -EBUSY;
> +
> + synquacer_i2c_hw_init(i2c);
> + bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
> + if (bsr & SYNQUACER_I2C_BSR_BB) {
> + dev_err(i2c->dev, "cannot get bus (bus busy)\n");
> + return -EBUSY;
> + }
> +
> + init_completion(>completion);

reinit_completion? And init_completion() in probe()?

> + /* ensure the stop has been through the bus */
> + bb_timeout = jiffies + HZ;
> + do {
> + bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
> + if (!(bsr & SYNQUACER_I2C_BSR_BB))
> + return 0;
> + } while (time_before(jiffies, bb_timeout));

Busy looping for one second? And won't the bus_free detection at the
beginning of a transfer do?

> +static int synquacer_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> +   int num)
> +{
> + struct synquacer_i2c *i2c;
> + int retry;
> + int ret;
> +
> + if (!msgs)
> + return -EINVAL;
> + if (num <= 0)
> + return -EINVAL;

Hmm, this should be done by the core. I am surprised it doesn't do that yet :/

> +
> + i2c = i2c_get_adapdata(adap);
> + i2c->timeout_ms = calc_timeout_ms(i2c, msgs, num);
> +
> + dev_dbg(i2c->dev, "calculated timeout %d ms\n", i2c->timeout_ms);
> +
> + for (retry = 0; retry < adap->retries; retry++) {
> +
> + ret = synquacer_i2c_doxfer(i2c, msgs, num);
> + if (ret != -EAGAIN)
> + return ret;
> +
> + dev_dbg(i2c->dev, "Retrying transmission (%d)\n", retry);
> +
> + synquacer_i2c_master_recover(i2c);

Recovery is only when SDA is stuck low, held by a client. That is
nothing you should do just on any error.

If you want the driver in v4.17, I'd suggest to drop
synquacer_i2c_master_recover() now and add it incrementally later, using
the existing recovery infrastructure. The rest is only minor stuff and
needs not much further discussion IMO.

Regards,

   Wolfram



signature.asc
Description: PGP signature

Re: [PATCH v5 2/2] i2c: add support for Socionext SynQuacer I2C controller

[Wolfram Sang]

Hi Ard,

> +static int synquacer_i2c_master_start(struct synquacer_i2c *i2c,
> +   struct i2c_msg *pmsg)
> +{
> + unsigned char bsr, bcr;
> +
> + if (pmsg->flags & I2C_M_RD)
> + writeb((pmsg->addr << 1) | 1,
> +i2c->base + SYNQUACER_I2C_REG_DAR);
> + else
> + writeb(pmsg->addr << 1,
> +i2c->base + SYNQUACER_I2C_REG_DAR);

writeb(i2c_8bit_addr_from_msg(pmsg), i2c->base + SYNQUACER_I2C_REG_DAR);

?

> +static int synquacer_i2c_master_recover(struct synquacer_i2c *i2c)
> +{

This is the bus recovery mechanism with toggling SCL pulses, right?
That should be implemented using a 'struct i2c_bus_recovery_info' and
the core helpers.

> +static int synquacer_i2c_doxfer(struct synquacer_i2c *i2c,
> + struct i2c_msg *msgs, int num)
> +{
> + unsigned char bsr;
> + unsigned long timeout, bb_timeout;
> + int ret;
> +
> + if (i2c->is_suspended)
> + return -EBUSY;
> +
> + synquacer_i2c_hw_init(i2c);
> + bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
> + if (bsr & SYNQUACER_I2C_BSR_BB) {
> + dev_err(i2c->dev, "cannot get bus (bus busy)\n");
> + return -EBUSY;
> + }
> +
> + init_completion(>completion);

reinit_completion? And init_completion() in probe()?

> + /* ensure the stop has been through the bus */
> + bb_timeout = jiffies + HZ;
> + do {
> + bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
> + if (!(bsr & SYNQUACER_I2C_BSR_BB))
> + return 0;
> + } while (time_before(jiffies, bb_timeout));

Busy looping for one second? And won't the bus_free detection at the
beginning of a transfer do?

> +static int synquacer_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> +   int num)
> +{
> + struct synquacer_i2c *i2c;
> + int retry;
> + int ret;
> +
> + if (!msgs)
> + return -EINVAL;
> + if (num <= 0)
> + return -EINVAL;

Hmm, this should be done by the core. I am surprised it doesn't do that yet :/

> +
> + i2c = i2c_get_adapdata(adap);
> + i2c->timeout_ms = calc_timeout_ms(i2c, msgs, num);
> +
> + dev_dbg(i2c->dev, "calculated timeout %d ms\n", i2c->timeout_ms);
> +
> + for (retry = 0; retry < adap->retries; retry++) {
> +
> + ret = synquacer_i2c_doxfer(i2c, msgs, num);
> + if (ret != -EAGAIN)
> + return ret;
> +
> + dev_dbg(i2c->dev, "Retrying transmission (%d)\n", retry);
> +
> + synquacer_i2c_master_recover(i2c);

Recovery is only when SDA is stuck low, held by a client. That is
nothing you should do just on any error.

If you want the driver in v4.17, I'd suggest to drop
synquacer_i2c_master_recover() now and add it incrementally later, using
the existing recovery infrastructure. The rest is only minor stuff and
needs not much further discussion IMO.

Regards,

   Wolfram



signature.asc
Description: PGP signature

Re: [PATCH 1/3] ieee80211: Replace bit shifts with the BIT() macro for WLAN_CAPABILITY_*.

[Quytelda Kahja]

The "document" refers to the file in which the changes were made
('include/linux/ieee80211.h').

I tend to try to split my commits into the smallest logically related
changes possible, hence the three patch series.  This particular case
may be a little on the extreme side, but if the maintainer desires,
they can always squash them together or ask me to resubmit as one
patch.

On 3/24/18, Larry Finger  wrote:
> On 03/23/2018 11:10 PM, Quytelda Kahja wrote:
>> It is neater and more consistent with the rest of the document to use the
>> BIT() macro from 'linux/bitops.h' to define the WLAN_CAPABILITY_*
>> bitmasks.  In the case of WLAN_CAPABILITY_DMG_TYPE_{IBSS, PBSS, AP},
>> bitshifting integers by 0 does nothing, so there is no reason to do it in
>> the code; replace these values with plain integers.
>>
>> Signed-off-by: Quytelda Kahja 
>
> In the commit message for all of these, what is the "document" to which you
> refer?
>
> I'm not quite sure why you split these changes into 3 parts, but I guess
> that is OK.
>
> Larry
>
-- 
Thank you,
Quytelda Kahja

Re: [PATCH 1/3] ieee80211: Replace bit shifts with the BIT() macro for WLAN_CAPABILITY_*.

[Quytelda Kahja]

The "document" refers to the file in which the changes were made
('include/linux/ieee80211.h').

I tend to try to split my commits into the smallest logically related
changes possible, hence the three patch series.  This particular case
may be a little on the extreme side, but if the maintainer desires,
they can always squash them together or ask me to resubmit as one
patch.

On 3/24/18, Larry Finger  wrote:
> On 03/23/2018 11:10 PM, Quytelda Kahja wrote:
>> It is neater and more consistent with the rest of the document to use the
>> BIT() macro from 'linux/bitops.h' to define the WLAN_CAPABILITY_*
>> bitmasks.  In the case of WLAN_CAPABILITY_DMG_TYPE_{IBSS, PBSS, AP},
>> bitshifting integers by 0 does nothing, so there is no reason to do it in
>> the code; replace these values with plain integers.
>>
>> Signed-off-by: Quytelda Kahja 
>
> In the commit message for all of these, what is the "document" to which you
> refer?
>
> I'm not quite sure why you split these changes into 3 parts, but I guess
> that is OK.
>
> Larry
>
-- 
Thank you,
Quytelda Kahja

Re: [PATCH v2 0/6] Add different features for I2C

[Wolfram Sang]

On Wed, Mar 21, 2018 at 05:48:54PM +0100, Pierre-Yves MORDRET wrote:
> Append new I2C STM32F7 feature set. This includes 10 bit support, slave
> support, SMBBus protocols support, DMA Support and eventually an I2C recovery
> mechanism.

So, I gave a few comments. For hardware details (especially DMA), some
additional review from Maxime or Alexandre would be great.



signature.asc
Description: PGP signature

Re: [PATCH v2 0/6] Add different features for I2C

[Wolfram Sang]

On Wed, Mar 21, 2018 at 05:48:54PM +0100, Pierre-Yves MORDRET wrote:
> Append new I2C STM32F7 feature set. This includes 10 bit support, slave
> support, SMBBus protocols support, DMA Support and eventually an I2C recovery
> mechanism.

So, I gave a few comments. For hardware details (especially DMA), some
additional review from Maxime or Alexandre would be great.



signature.asc
Description: PGP signature

Re: [PATCH v2 6/6] i2c: i2c-stm32f7: Implement I2C recovery mechanism

[Wolfram Sang]

On Wed, Mar 21, 2018 at 05:49:00PM +0100, Pierre-Yves MORDRET wrote:
> Feature prevents I2C lock-ups. Mechanism resets I2C state machine
> and releases SCL/SDA signals but preserves I2C registers.
> 
> Signed-off-by: Pierre-Yves MORDRET 
> ---
>   Version history:
> v1:
>* Initial
> v2:
>* Don't use i2c engine recovery mechanism since driver
>  procedure only recover master and not the slave.

s/recovery/release/ throughout the patch, please. Recovery is really
something else. Also, I think the dev_info's are too noisy in the log
files. I'd think the whole driver could be lifted from quite some
logging...

> ---
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 27 ---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c 
> b/drivers/i2c/busses/i2c-stm32f7.c
> index 91f73e0..9a9c469 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -718,6 +718,20 @@ static void stm32f7_i2c_smbus_reload(struct 
> stm32f7_i2c_dev *i2c_dev)
>   writel_relaxed(cr2, i2c_dev->base + STM32F7_I2C_CR2);
>  }
>  
> +static int stm32f7_i2c_recover_bus(struct i2c_adapter *i2c_adap)
> +{
> + struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
> +
> + dev_info(i2c_dev->dev, "Trying to recover bus\n");
> +
> + stm32f7_i2c_clr_bits(i2c_dev->base + STM32F7_I2C_CR1,
> +  STM32F7_I2C_CR1_PE);
> +
> + stm32f7_i2c_hw_config(i2c_dev);
> +
> + return 0;
> +}
> +
>  static int stm32f7_i2c_wait_free_bus(struct stm32f7_i2c_dev *i2c_dev)
>  {
>   u32 status;
> @@ -727,12 +741,18 @@ static int stm32f7_i2c_wait_free_bus(struct 
> stm32f7_i2c_dev *i2c_dev)
>status,
>!(status & STM32F7_I2C_ISR_BUSY),
>10, 1000);
> + if (!ret)
> + return 0;
> +
> + dev_info(i2c_dev->dev, "bus busy\n");
> +
> + ret = stm32f7_i2c_recover_bus(_dev->adap);
>   if (ret) {
> - dev_dbg(i2c_dev->dev, "bus busy\n");
> - ret = -EBUSY;
> + dev_err(i2c_dev->dev, "Failed to recover the bus (%d)\n", ret);
> + return ret;
>   }
>  
> - return ret;
> + return -EBUSY;
>  }
>  
>  static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
> @@ -1474,6 +1494,7 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void 
> *data)
>   if (status & STM32F7_I2C_ISR_BERR) {
>   dev_err(dev, "<%s>: Bus error\n", __func__);
>   writel_relaxed(STM32F7_I2C_ICR_BERRCF, base + STM32F7_I2C_ICR);
> + stm32f7_i2c_recover_bus(_dev->adap);
>   f7_msg->result = -EIO;
>   }
>  
> -- 
> 2.7.4
> 


signature.asc
Description: PGP signature

Re: [PATCH v2 6/6] i2c: i2c-stm32f7: Implement I2C recovery mechanism

[Wolfram Sang]

On Wed, Mar 21, 2018 at 05:49:00PM +0100, Pierre-Yves MORDRET wrote:
> Feature prevents I2C lock-ups. Mechanism resets I2C state machine
> and releases SCL/SDA signals but preserves I2C registers.
> 
> Signed-off-by: Pierre-Yves MORDRET 
> ---
>   Version history:
> v1:
>* Initial
> v2:
>* Don't use i2c engine recovery mechanism since driver
>  procedure only recover master and not the slave.

s/recovery/release/ throughout the patch, please. Recovery is really
something else. Also, I think the dev_info's are too noisy in the log
files. I'd think the whole driver could be lifted from quite some
logging...

> ---
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 27 ---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c 
> b/drivers/i2c/busses/i2c-stm32f7.c
> index 91f73e0..9a9c469 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -718,6 +718,20 @@ static void stm32f7_i2c_smbus_reload(struct 
> stm32f7_i2c_dev *i2c_dev)
>   writel_relaxed(cr2, i2c_dev->base + STM32F7_I2C_CR2);
>  }
>  
> +static int stm32f7_i2c_recover_bus(struct i2c_adapter *i2c_adap)
> +{
> + struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
> +
> + dev_info(i2c_dev->dev, "Trying to recover bus\n");
> +
> + stm32f7_i2c_clr_bits(i2c_dev->base + STM32F7_I2C_CR1,
> +  STM32F7_I2C_CR1_PE);
> +
> + stm32f7_i2c_hw_config(i2c_dev);
> +
> + return 0;
> +}
> +
>  static int stm32f7_i2c_wait_free_bus(struct stm32f7_i2c_dev *i2c_dev)
>  {
>   u32 status;
> @@ -727,12 +741,18 @@ static int stm32f7_i2c_wait_free_bus(struct 
> stm32f7_i2c_dev *i2c_dev)
>status,
>!(status & STM32F7_I2C_ISR_BUSY),
>10, 1000);
> + if (!ret)
> + return 0;
> +
> + dev_info(i2c_dev->dev, "bus busy\n");
> +
> + ret = stm32f7_i2c_recover_bus(_dev->adap);
>   if (ret) {
> - dev_dbg(i2c_dev->dev, "bus busy\n");
> - ret = -EBUSY;
> + dev_err(i2c_dev->dev, "Failed to recover the bus (%d)\n", ret);
> + return ret;
>   }
>  
> - return ret;
> + return -EBUSY;
>  }
>  
>  static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
> @@ -1474,6 +1494,7 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void 
> *data)
>   if (status & STM32F7_I2C_ISR_BERR) {
>   dev_err(dev, "<%s>: Bus error\n", __func__);
>   writel_relaxed(STM32F7_I2C_ICR_BERRCF, base + STM32F7_I2C_ICR);
> + stm32f7_i2c_recover_bus(_dev->adap);
>   f7_msg->result = -EIO;
>   }
>  
> -- 
> 2.7.4
> 


signature.asc
Description: PGP signature

Re: [PATCH v2 4/6] i2c: i2c-stm32: Add generic DMA API

[Wolfram Sang]

On Wed, Mar 21, 2018 at 05:48:58PM +0100, Pierre-Yves MORDRET wrote:
> This patch adds a generic DMA API to implement DMA support for i2c-stm32fx
> drivers
> 
> Signed-off-by: M'boumba Cedric Madianga 
> Signed-off-by: Pierre-Yves MORDRET 

In case you haven't read it so far, there is a new document about I2C &
DMA -> Documentation/i2c/DMA-considerations

Just so you know...



signature.asc
Description: PGP signature

Re: [PATCH v2 4/6] i2c: i2c-stm32: Add generic DMA API

[Wolfram Sang]

On Wed, Mar 21, 2018 at 05:48:58PM +0100, Pierre-Yves MORDRET wrote:
> This patch adds a generic DMA API to implement DMA support for i2c-stm32fx
> drivers
> 
> Signed-off-by: M'boumba Cedric Madianga 
> Signed-off-by: Pierre-Yves MORDRET 

In case you haven't read it so far, there is a new document about I2C &
DMA -> Documentation/i2c/DMA-considerations

Just so you know...



signature.asc
Description: PGP signature

Re: [PATCH v2 3/6] i2c: i2c-stm32f7: Add initial SMBus protocols support

[Wolfram Sang]

On Wed, Mar 21, 2018 at 05:48:57PM +0100, Pierre-Yves MORDRET wrote:
> This patch adds SMBus support for I2C controller embedded in STM32F7 Soc.

> All SMBus protocols are implemented except SMBus-specific protocols.

What does that mean?

> 
> Signed-off-by: M'boumba Cedric Madianga 
> Signed-off-by: Pierre-Yves MORDRET 
> ---
>   Version history:
> v1:
>* Initial
> v2:
>* fix Kbuild test robot issue (Unneeded semicolon)
> ---
> 
> fixup! i2c: i2c-stm32f7: Add initial SMBus protocols support
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 377 
> ++-
>  1 file changed, 368 insertions(+), 9 deletions(-)

That is quite some complexity considering we have I2C_FUNC_SMBUS_EMUL. I
don't mind, but you really want that?



signature.asc
Description: PGP signature

Re: [PATCH v2 3/6] i2c: i2c-stm32f7: Add initial SMBus protocols support

[Wolfram Sang]

On Wed, Mar 21, 2018 at 05:48:57PM +0100, Pierre-Yves MORDRET wrote:
> This patch adds SMBus support for I2C controller embedded in STM32F7 Soc.

> All SMBus protocols are implemented except SMBus-specific protocols.

What does that mean?

> 
> Signed-off-by: M'boumba Cedric Madianga 
> Signed-off-by: Pierre-Yves MORDRET 
> ---
>   Version history:
> v1:
>* Initial
> v2:
>* fix Kbuild test robot issue (Unneeded semicolon)
> ---
> 
> fixup! i2c: i2c-stm32f7: Add initial SMBus protocols support
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 377 
> ++-
>  1 file changed, 368 insertions(+), 9 deletions(-)

That is quite some complexity considering we have I2C_FUNC_SMBUS_EMUL. I
don't mind, but you really want that?



signature.asc
Description: PGP signature

Charity

[Alice Walton]

I have a mission worth $ 100,000,000.00 for you

Charity

[Alice Walton]

I have a mission worth $ 100,000,000.00 for you

[PATCH] usb: host: Remove the deprecated ATH79 USB host config options

[Alban Bedel]

The options USB_EHCI_ATH79 and USB_OHCI_ATH79 only enable the
generic EHCI and OHCI platform drivers, and have been marked as
deprecated since 2012.

These can be safely removed if we make sure that USB_EHCI_ROOT_HUB_TT
still get enabled for the EHCI driver. This is now done be selecting
this option when the EHCI platform driver is enabled on the ATH79
platform.

Signed-off-by: Alban Bedel 
---
 arch/mips/Kconfig|  1 +
 drivers/usb/host/Kconfig | 25 -
 2 files changed, 1 insertion(+), 25 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 8128c3b..61e9a24 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -200,6 +200,7 @@ config ATH79
select SYS_SUPPORTS_MIPS16
select SYS_SUPPORTS_ZBOOT_UART_PROM
select USE_OF
+   select USB_EHCI_ROOT_HUB_TT if USB_EHCI_HCD_PLATFORM
help
  Support for the Atheros AR71XX/AR724X/AR913X SoCs.
 
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 4fcfb30..55b45dc 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -293,19 +293,6 @@ config USB_CNS3XXX_EHCI
  It is needed for high-speed (480Mbit/sec) USB 2.0 device
  support.
 
-config USB_EHCI_ATH79
-   bool "EHCI support for AR7XXX/AR9XXX SoCs (DEPRECATED)"
-   depends on (SOC_AR71XX || SOC_AR724X || SOC_AR913X || SOC_AR933X)
-   select USB_EHCI_ROOT_HUB_TT
-   select USB_EHCI_HCD_PLATFORM
-   default y
-   ---help---
- This option is deprecated now and the driver was removed, use
- USB_EHCI_HCD_PLATFORM instead.
-
- Enables support for the built-in EHCI controller present
- on the Atheros AR7XXX/AR9XXX SoCs.
-
 config USB_EHCI_HCD_PLATFORM
tristate "Generic EHCI driver for a platform device"
default n
@@ -489,18 +476,6 @@ config USB_OHCI_HCD_DAVINCI
  controller. This driver cannot currently be a loadable
  module because it lacks a proper PHY abstraction.
 
-config USB_OHCI_ATH79
-   bool "USB OHCI support for the Atheros AR71XX/AR7240 SoCs (DEPRECATED)"
-   depends on (SOC_AR71XX || SOC_AR724X)
-   select USB_OHCI_HCD_PLATFORM
-   default y
-   help
- This option is deprecated now and the driver was removed, use
- USB_OHCI_HCD_PLATFORM instead.
-
- Enables support for the built-in OHCI controller present on the
- Atheros AR71XX/AR7240 SoCs.
-
 config USB_OHCI_HCD_PPC_OF_BE
bool "OHCI support for OF platform bus (big endian)"
depends on PPC
-- 
2.7.4

[PATCH] usb: host: Remove the deprecated ATH79 USB host config options

[Alban Bedel]

The options USB_EHCI_ATH79 and USB_OHCI_ATH79 only enable the
generic EHCI and OHCI platform drivers, and have been marked as
deprecated since 2012.

These can be safely removed if we make sure that USB_EHCI_ROOT_HUB_TT
still get enabled for the EHCI driver. This is now done be selecting
this option when the EHCI platform driver is enabled on the ATH79
platform.

Signed-off-by: Alban Bedel 
---
 arch/mips/Kconfig|  1 +
 drivers/usb/host/Kconfig | 25 -
 2 files changed, 1 insertion(+), 25 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 8128c3b..61e9a24 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -200,6 +200,7 @@ config ATH79
select SYS_SUPPORTS_MIPS16
select SYS_SUPPORTS_ZBOOT_UART_PROM
select USE_OF
+   select USB_EHCI_ROOT_HUB_TT if USB_EHCI_HCD_PLATFORM
help
  Support for the Atheros AR71XX/AR724X/AR913X SoCs.
 
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 4fcfb30..55b45dc 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -293,19 +293,6 @@ config USB_CNS3XXX_EHCI
  It is needed for high-speed (480Mbit/sec) USB 2.0 device
  support.
 
-config USB_EHCI_ATH79
-   bool "EHCI support for AR7XXX/AR9XXX SoCs (DEPRECATED)"
-   depends on (SOC_AR71XX || SOC_AR724X || SOC_AR913X || SOC_AR933X)
-   select USB_EHCI_ROOT_HUB_TT
-   select USB_EHCI_HCD_PLATFORM
-   default y
-   ---help---
- This option is deprecated now and the driver was removed, use
- USB_EHCI_HCD_PLATFORM instead.
-
- Enables support for the built-in EHCI controller present
- on the Atheros AR7XXX/AR9XXX SoCs.
-
 config USB_EHCI_HCD_PLATFORM
tristate "Generic EHCI driver for a platform device"
default n
@@ -489,18 +476,6 @@ config USB_OHCI_HCD_DAVINCI
  controller. This driver cannot currently be a loadable
  module because it lacks a proper PHY abstraction.
 
-config USB_OHCI_ATH79
-   bool "USB OHCI support for the Atheros AR71XX/AR7240 SoCs (DEPRECATED)"
-   depends on (SOC_AR71XX || SOC_AR724X)
-   select USB_OHCI_HCD_PLATFORM
-   default y
-   help
- This option is deprecated now and the driver was removed, use
- USB_OHCI_HCD_PLATFORM instead.
-
- Enables support for the built-in OHCI controller present on the
- Atheros AR71XX/AR7240 SoCs.
-
 config USB_OHCI_HCD_PPC_OF_BE
bool "OHCI support for OF platform bus (big endian)"
depends on PPC
-- 
2.7.4

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

> On Mar 24, 2018, at 8:02 AM, Jonathan Cameron  wrote:
> 
> On Mon, 19 Mar 2018 22:57:16 -0700
> John Syne  wrote:
> 
>> Hi Jonathan,
>> 
>> Thank you for all your hard work. Your feedback is really helpful. I’m 
>> surprised that no one from Analog Device has offered any suggestions.
>> 
> 
> Sadly those active in the mainline linux kernel from ADI are focused in
> other areas currently :(
Good point. I will reach out to Analog Devices and get a name of someone 
who is knowledgeable in this area. Perhaps Lars-Peters has a suggestion. 
>> Anyway, please see my comments inline below
>> 
>> Regards,
>> John
>> 
>> 
>> 
>> 
>> 
>>> On Mar 18, 2018, at 5:23 AM, Jonathan Cameron  wrote:
>>> 
>>> On Sat, 17 Mar 2018 23:11:45 -0700
>>> John Syne  wrote:
>>> 
 Hi Jonathan,  
>>> Hi John and All,
>>> 
>>> I'd love to get some additional input on this from anyone interested.
>>> There are a lot of weird and wonderful derived quantities in an energy
>>> meter and it seems we need to make some fundamental changes to support
>>> them - including potentially a userspace breaking change to the event
>>> code description.
>>> 
 
 Here is the complete list of registers for the ADE7878 which I copied from 
 the data sheet. I added a column “IIO Attribute” which I hope follows your 
 IIO ABI. Please make any changes you feel are incorrect. BTW, there are 
 several registers that cannot be generalized and are used purely for chip 
 configuration. I think we should add a new naming convention, namely 
 {register}_{}. Also, I see in the sys_bus_iio doc
> 
> 
> 
> 
 in_accel_x_peak_raw
 
 so shouldn’t the phase be represented as follows:
 
 in_current_A_scale  
>>> I'm still confused.  What does A represent here?  I assumed that was a wild
>>> card for the channel number before but clearly not.
>>> 
>>> Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
>>> I guess they sort of look like axis, and sort of like independent channels.
>>> So could be indexed or done via modifiers depending on how you look at it.  
>> In metering terminology, the three phases are commonly referred to as RED, 
>> GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices uses the 
>> ABCN nomenclature so anyone using this driver will probably have referenced 
>> the Analog Devices datasheet so this terminology should be familiar. 
> Which is more commonly used in general?  We are designing what we hope
> will be a general interface and last thing we want is to have everyone
> not using ADI parts confused!  Personally not assigning 'colours' makes
> more sense to me.
I did a little research and here is the naming used by vendor:

ST Microelectronics: P1, P2, P3, N
http://www.st.com/content/ccc/resource/technical/document/application_note/5e/f4/22/4d/90/96/4c/c4/CD00153264.pdf/files/CD00153264.pdf/jcr:content/translations/en.CD00153264.pdf

Teridian: ABCN
https://www.mouser.com/ds/2/256/71M6513-71M6513H-22603.pdf

Maxim Integrated: ABCN
https://datasheets.maximintegrated.com/en/ds/78M6631.pdf

Microchip: ABCN
http://ww1.microchip.com/downloads/en/DeviceDoc/51643b.pdf

NXP: L1, L2, L3, N
https://www.nxp.com/support/developer-resources/reference-designs/kinetis-km3x-256-mcu-three-phase-metering-reference-design:RD-KM3x-256-3PH-METERING

Renesas: ABCN
https://www.renesas.com/en-us/solutions/home/metering/power-meter-three.html

So the most consistent would be ABCN


All vendor providing three phase energy metering ICs use the ABCN terminology. 
> 
> (btw please wrap any lines that don't need to be long to around 80 
> characters).
> 
>>> 
>>> Hmm. With neutral in there as well I guess we need to make them
>>> modifiers (but might change my mind later ;)
>>> 
>>> Particularly as we are using the the modifier for RMS under the previous
>>> plan... It appears we should treat that instead like we did for peak
>>> and do it as an additional info mask element.  The problem with doing that
>>> on a continuous measurement is that we can't treat it as a channel to
>>> be output through the buffered interface.  
>> I’ve changed the layout of the spreadsheet to breakout the Direction, Type, 
>> Index, Modifier and Info Mask to make sure there is no miss-understanding 
>> and will help identify all the items we need to add.
>> 
>> The ADE7878 channels that will use the buffer are IAWV, VAWV, IBWV, VBWV, 
>> ICWV, VCWV, INWV, AVA, BVA, CVA, AWATT, BWATT, CWATT, AVAR, BVAR, and CVAR. 
>> So I guess we have to add an index
> Probably a good idea anyway, we are sort of slowly moving away
> from index less channels.  The ABI always allowed index assignment
> and it makes for easier userspace code.
> 
>> 
>> Address Register IIO Attribute   
>> Dir TypeIndex   ModifierInfo Mask   R/W  
>>Bit Bit Length  TypeDefault 

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

> On Mar 24, 2018, at 8:02 AM, Jonathan Cameron  wrote:
> 
> On Mon, 19 Mar 2018 22:57:16 -0700
> John Syne  wrote:
> 
>> Hi Jonathan,
>> 
>> Thank you for all your hard work. Your feedback is really helpful. I’m 
>> surprised that no one from Analog Device has offered any suggestions.
>> 
> 
> Sadly those active in the mainline linux kernel from ADI are focused in
> other areas currently :(
Good point. I will reach out to Analog Devices and get a name of someone 
who is knowledgeable in this area. Perhaps Lars-Peters has a suggestion. 
>> Anyway, please see my comments inline below
>> 
>> Regards,
>> John
>> 
>> 
>> 
>> 
>> 
>>> On Mar 18, 2018, at 5:23 AM, Jonathan Cameron  wrote:
>>> 
>>> On Sat, 17 Mar 2018 23:11:45 -0700
>>> John Syne  wrote:
>>> 
 Hi Jonathan,  
>>> Hi John and All,
>>> 
>>> I'd love to get some additional input on this from anyone interested.
>>> There are a lot of weird and wonderful derived quantities in an energy
>>> meter and it seems we need to make some fundamental changes to support
>>> them - including potentially a userspace breaking change to the event
>>> code description.
>>> 
 
 Here is the complete list of registers for the ADE7878 which I copied from 
 the data sheet. I added a column “IIO Attribute” which I hope follows your 
 IIO ABI. Please make any changes you feel are incorrect. BTW, there are 
 several registers that cannot be generalized and are used purely for chip 
 configuration. I think we should add a new naming convention, namely 
 {register}_{}. Also, I see in the sys_bus_iio doc
> 
> 
> 
> 
 in_accel_x_peak_raw
 
 so shouldn’t the phase be represented as follows:
 
 in_current_A_scale  
>>> I'm still confused.  What does A represent here?  I assumed that was a wild
>>> card for the channel number before but clearly not.
>>> 
>>> Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
>>> I guess they sort of look like axis, and sort of like independent channels.
>>> So could be indexed or done via modifiers depending on how you look at it.  
>> In metering terminology, the three phases are commonly referred to as RED, 
>> GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices uses the 
>> ABCN nomenclature so anyone using this driver will probably have referenced 
>> the Analog Devices datasheet so this terminology should be familiar. 
> Which is more commonly used in general?  We are designing what we hope
> will be a general interface and last thing we want is to have everyone
> not using ADI parts confused!  Personally not assigning 'colours' makes
> more sense to me.
I did a little research and here is the naming used by vendor:

ST Microelectronics: P1, P2, P3, N
http://www.st.com/content/ccc/resource/technical/document/application_note/5e/f4/22/4d/90/96/4c/c4/CD00153264.pdf/files/CD00153264.pdf/jcr:content/translations/en.CD00153264.pdf

Teridian: ABCN
https://www.mouser.com/ds/2/256/71M6513-71M6513H-22603.pdf

Maxim Integrated: ABCN
https://datasheets.maximintegrated.com/en/ds/78M6631.pdf

Microchip: ABCN
http://ww1.microchip.com/downloads/en/DeviceDoc/51643b.pdf

NXP: L1, L2, L3, N
https://www.nxp.com/support/developer-resources/reference-designs/kinetis-km3x-256-mcu-three-phase-metering-reference-design:RD-KM3x-256-3PH-METERING

Renesas: ABCN
https://www.renesas.com/en-us/solutions/home/metering/power-meter-three.html

So the most consistent would be ABCN


All vendor providing three phase energy metering ICs use the ABCN terminology. 
> 
> (btw please wrap any lines that don't need to be long to around 80 
> characters).
> 
>>> 
>>> Hmm. With neutral in there as well I guess we need to make them
>>> modifiers (but might change my mind later ;)
>>> 
>>> Particularly as we are using the the modifier for RMS under the previous
>>> plan... It appears we should treat that instead like we did for peak
>>> and do it as an additional info mask element.  The problem with doing that
>>> on a continuous measurement is that we can't treat it as a channel to
>>> be output through the buffered interface.  
>> I’ve changed the layout of the spreadsheet to breakout the Direction, Type, 
>> Index, Modifier and Info Mask to make sure there is no miss-understanding 
>> and will help identify all the items we need to add.
>> 
>> The ADE7878 channels that will use the buffer are IAWV, VAWV, IBWV, VBWV, 
>> ICWV, VCWV, INWV, AVA, BVA, CVA, AWATT, BWATT, CWATT, AVAR, BVAR, and CVAR. 
>> So I guess we have to add an index
> Probably a good idea anyway, we are sort of slowly moving away
> from index less channels.  The ABI always allowed index assignment
> and it makes for easier userspace code.
> 
>> 
>> Address Register IIO Attribute   
>> Dir TypeIndex   ModifierInfo Mask   R/W  
>>Bit Bit Length  TypeDefault Description 
>>

Re: [PATCH v2 1/6] i2c: i2c-stm32f7: Add 10-bit address support

[Wolfram Sang]

On Wed, Mar 21, 2018 at 05:48:55PM +0100, Pierre-Yves MORDRET wrote:
> This patch adds support for 10-bit device address for STM32F7 I2C
> 
> Signed-off-by: M'boumba Cedric Madianga 
> Signed-off-by: Pierre-Yves MORDRET 

Out of curiosity: how did you test this patch? I never managed to find a
10-bit client (except for an SoC with 10-bit slave mode).

> ---
>   Version history:
>  v1:
> * Initial
>  v2:
> ---
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 22 +-
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c 
> b/drivers/i2c/busses/i2c-stm32f7.c
> index f273e28..ae0d15c 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -65,7 +65,12 @@
>  #define STM32F7_I2C_CR2_NACK BIT(15)
>  #define STM32F7_I2C_CR2_STOP BIT(14)
>  #define STM32F7_I2C_CR2_STARTBIT(13)
> +#define STM32F7_I2C_CR2_HEAD10R  BIT(12)
> +#define STM32F7_I2C_CR2_ADD10BIT(11)
>  #define STM32F7_I2C_CR2_RD_WRN   BIT(10)
> +#define STM32F7_I2C_CR2_SADD10_MASK  GENMASK(9, 0)
> +#define STM32F7_I2C_CR2_SADD10(n)(((n) & \
> + STM32F7_I2C_CR2_SADD10_MASK))
>  #define STM32F7_I2C_CR2_SADD7_MASK   GENMASK(7, 1)
>  #define STM32F7_I2C_CR2_SADD7(n) (((n) & 0x7f) << 1)
>  
> @@ -176,14 +181,14 @@ struct stm32f7_i2c_timings {
>  
>  /**
>   * struct stm32f7_i2c_msg - client specific data
> - * @addr: 8-bit slave addr, including r/w bit
> + * @addr: 8-bit or 10-bit slave addr, including r/w bit
>   * @count: number of bytes to be transferred
>   * @buf: data buffer
>   * @result: result of the transfer
>   * @stop: last I2C msg to be sent, i.e. STOP to be generated
>   */
>  struct stm32f7_i2c_msg {
> - u8 addr;
> + u16 addr;
>   u32 count;
>   u8 *buf;
>   int result;
> @@ -629,8 +634,15 @@ static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev 
> *i2c_dev,
>   cr2 |= STM32F7_I2C_CR2_RD_WRN;
>  
>   /* Set slave address */
> - cr2 &= ~STM32F7_I2C_CR2_SADD7_MASK;
> - cr2 |= STM32F7_I2C_CR2_SADD7(f7_msg->addr);
> + cr2 &= ~(STM32F7_I2C_CR2_HEAD10R | STM32F7_I2C_CR2_ADD10);
> + if (msg->flags & I2C_M_TEN) {
> + cr2 &= ~STM32F7_I2C_CR2_SADD10_MASK;
> + cr2 |= STM32F7_I2C_CR2_SADD10(f7_msg->addr);
> + cr2 |= STM32F7_I2C_CR2_ADD10;
> + } else {
> + cr2 &= ~STM32F7_I2C_CR2_SADD7_MASK;
> + cr2 |= STM32F7_I2C_CR2_SADD7(f7_msg->addr);
> + }
>  
>   /* Set nb bytes to transfer and reload if needed */
>   cr2 &= ~(STM32F7_I2C_CR2_NBYTES_MASK | STM32F7_I2C_CR2_RELOAD);
> @@ -798,7 +810,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
>  
>  static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
>  {
> - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR;
>  }
>  
>  static struct i2c_algorithm stm32f7_i2c_algo = {
> -- 
> 2.7.4
> 


signature.asc
Description: PGP signature

Re: [PATCH v2 1/6] i2c: i2c-stm32f7: Add 10-bit address support

[Wolfram Sang]

On Wed, Mar 21, 2018 at 05:48:55PM +0100, Pierre-Yves MORDRET wrote:
> This patch adds support for 10-bit device address for STM32F7 I2C
> 
> Signed-off-by: M'boumba Cedric Madianga 
> Signed-off-by: Pierre-Yves MORDRET 

Out of curiosity: how did you test this patch? I never managed to find a
10-bit client (except for an SoC with 10-bit slave mode).

> ---
>   Version history:
>  v1:
> * Initial
>  v2:
> ---
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 22 +-
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c 
> b/drivers/i2c/busses/i2c-stm32f7.c
> index f273e28..ae0d15c 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -65,7 +65,12 @@
>  #define STM32F7_I2C_CR2_NACK BIT(15)
>  #define STM32F7_I2C_CR2_STOP BIT(14)
>  #define STM32F7_I2C_CR2_STARTBIT(13)
> +#define STM32F7_I2C_CR2_HEAD10R  BIT(12)
> +#define STM32F7_I2C_CR2_ADD10BIT(11)
>  #define STM32F7_I2C_CR2_RD_WRN   BIT(10)
> +#define STM32F7_I2C_CR2_SADD10_MASK  GENMASK(9, 0)
> +#define STM32F7_I2C_CR2_SADD10(n)(((n) & \
> + STM32F7_I2C_CR2_SADD10_MASK))
>  #define STM32F7_I2C_CR2_SADD7_MASK   GENMASK(7, 1)
>  #define STM32F7_I2C_CR2_SADD7(n) (((n) & 0x7f) << 1)
>  
> @@ -176,14 +181,14 @@ struct stm32f7_i2c_timings {
>  
>  /**
>   * struct stm32f7_i2c_msg - client specific data
> - * @addr: 8-bit slave addr, including r/w bit
> + * @addr: 8-bit or 10-bit slave addr, including r/w bit
>   * @count: number of bytes to be transferred
>   * @buf: data buffer
>   * @result: result of the transfer
>   * @stop: last I2C msg to be sent, i.e. STOP to be generated
>   */
>  struct stm32f7_i2c_msg {
> - u8 addr;
> + u16 addr;
>   u32 count;
>   u8 *buf;
>   int result;
> @@ -629,8 +634,15 @@ static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev 
> *i2c_dev,
>   cr2 |= STM32F7_I2C_CR2_RD_WRN;
>  
>   /* Set slave address */
> - cr2 &= ~STM32F7_I2C_CR2_SADD7_MASK;
> - cr2 |= STM32F7_I2C_CR2_SADD7(f7_msg->addr);
> + cr2 &= ~(STM32F7_I2C_CR2_HEAD10R | STM32F7_I2C_CR2_ADD10);
> + if (msg->flags & I2C_M_TEN) {
> + cr2 &= ~STM32F7_I2C_CR2_SADD10_MASK;
> + cr2 |= STM32F7_I2C_CR2_SADD10(f7_msg->addr);
> + cr2 |= STM32F7_I2C_CR2_ADD10;
> + } else {
> + cr2 &= ~STM32F7_I2C_CR2_SADD7_MASK;
> + cr2 |= STM32F7_I2C_CR2_SADD7(f7_msg->addr);
> + }
>  
>   /* Set nb bytes to transfer and reload if needed */
>   cr2 &= ~(STM32F7_I2C_CR2_NBYTES_MASK | STM32F7_I2C_CR2_RELOAD);
> @@ -798,7 +810,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
>  
>  static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
>  {
> - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR;
>  }
>  
>  static struct i2c_algorithm stm32f7_i2c_algo = {
> -- 
> 2.7.4
> 


signature.asc
Description: PGP signature

[PATCH] phy: Add a driver for the ATH79 USB phy

[Alban Bedel]

The ATH79 USB phy is very simple, it only have a reset. On some SoC a
second reset is used to force the phy in suspend mode regardless of the
USB controller status.

This driver is added to the qualcom directory as atheros is now part
of qualcom and newer SoC of this familly are marketed under the
qualcom name.

Signed-off-by: Alban Bedel 
---
 MAINTAINERS  |   8 +++
 drivers/phy/qualcomm/Kconfig |  11 +++-
 drivers/phy/qualcomm/Makefile|   1 +
 drivers/phy/qualcomm/phy-ath79-usb.c | 108 +++
 4 files changed, 127 insertions(+), 1 deletion(-)
 create mode 100644 drivers/phy/qualcomm/phy-ath79-usb.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 73c0cda..ce5ecd6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2294,6 +2294,14 @@ S:   Maintained
 F: drivers/gpio/gpio-ath79.c
 F: Documentation/devicetree/bindings/gpio/gpio-ath79.txt
 
+ATHEROS 71XX/9XXX USB PHY DRIVER
+M: Alban Bedel 
+W: https://github.com/AlbanBedel/linux
+T: git git://github.com/AlbanBedel/linux
+S: Maintained
+F: drivers/phy/qualcomm/phy-ath79-usb.c
+F: Documentation/devicetree/bindings/phy/phy-ath79-usb.txt
+
 ATHEROS ATH GENERIC UTILITIES
 M: "Luis R. Rodriguez" 
 L: linux-wirel...@vger.kernel.org
diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index 7bfa64b..632a0e7 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -1,6 +1,15 @@
 #
-# Phy drivers for Qualcomm platforms
+# Phy drivers for Qualcomm and Atheros platforms
 #
+config PHY_ATH79_USB
+   tristate "Atheros AR71XX/9XXX USB PHY driver"
+   depends on OF && (ATH79 || COMPILE_TEST)
+   default y if USB_EHCI_HCD_PLATFORM || USB_OHCI_HCD_PLATFORM
+   select RESET_CONTROLLER
+   select GENERIC_PHY
+   help
+ Enable this to support the USB PHY on Atheros AR71XX/9XXX SoCs.
+
 config PHY_QCOM_APQ8064_SATA
tristate "Qualcomm APQ8064 SATA SerDes/PHY driver"
depends on ARCH_QCOM
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index 9abb789..deb831f4 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_PHY_ATH79_USB)+= phy-ath79-usb.o
 obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)+= phy-qcom-apq8064-sata.o
 obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)+= phy-qcom-ipq806x-sata.o
 obj-$(CONFIG_PHY_QCOM_QMP) += phy-qcom-qmp.o
diff --git a/drivers/phy/qualcomm/phy-ath79-usb.c 
b/drivers/phy/qualcomm/phy-ath79-usb.c
new file mode 100644
index 000..6fd6e07
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-ath79-usb.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Atheros AR71XX/9XXX USB PHY driver
+ *
+ * Copyright (C) 2015-2018 Alban Bedel 
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+struct ath79_usb_phy {
+   struct reset_control *reset;
+   /* The suspend override logic is inverted, hence the no prefix
+* to make the code a bit easier to understand.
+*/
+   struct reset_control *no_suspend_override;
+};
+
+static int ath79_usb_phy_power_on(struct phy *phy)
+{
+   struct ath79_usb_phy *priv = phy_get_drvdata(phy);
+   int err = 0;
+
+   if (priv->no_suspend_override) {
+   err = reset_control_assert(priv->no_suspend_override);
+   if (err)
+   return err;
+   }
+
+   err = reset_control_deassert(priv->reset);
+   if (err && priv->no_suspend_override)
+   reset_control_assert(priv->no_suspend_override);
+
+   return err;
+}
+
+static int ath79_usb_phy_power_off(struct phy *phy)
+{
+   struct ath79_usb_phy *priv = phy_get_drvdata(phy);
+   int err = 0;
+
+   err = reset_control_assert(priv->reset);
+   if (err)
+   return err;
+
+   if (priv->no_suspend_override) {
+   err = reset_control_deassert(priv->no_suspend_override);
+   if (err)
+   reset_control_deassert(priv->reset);
+   }
+
+   return err;
+}
+
+static const struct phy_ops ath79_usb_phy_ops = {
+   .power_on   = ath79_usb_phy_power_on,
+   .power_off  = ath79_usb_phy_power_off,
+   .owner  = THIS_MODULE,
+};
+
+static int ath79_usb_phy_probe(struct platform_device *pdev)
+{
+   struct ath79_usb_phy *priv;
+   struct phy *phy;
+
+   priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
+   if (!priv)
+   return -ENOMEM;
+
+   priv->reset = devm_reset_control_get(>dev, "usb-phy");
+   if (IS_ERR(priv->reset))
+   return PTR_ERR(priv->reset);
+
+   priv->no_suspend_override = devm_reset_control_get_optional(
+   >dev, "usb-suspend-override");
+   if (IS_ERR(priv->no_suspend_override))
+   return 

[PATCH] phy: Add a driver for the ATH79 USB phy

[Alban Bedel]

The ATH79 USB phy is very simple, it only have a reset. On some SoC a
second reset is used to force the phy in suspend mode regardless of the
USB controller status.

This driver is added to the qualcom directory as atheros is now part
of qualcom and newer SoC of this familly are marketed under the
qualcom name.

Signed-off-by: Alban Bedel 
---
 MAINTAINERS  |   8 +++
 drivers/phy/qualcomm/Kconfig |  11 +++-
 drivers/phy/qualcomm/Makefile|   1 +
 drivers/phy/qualcomm/phy-ath79-usb.c | 108 +++
 4 files changed, 127 insertions(+), 1 deletion(-)
 create mode 100644 drivers/phy/qualcomm/phy-ath79-usb.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 73c0cda..ce5ecd6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2294,6 +2294,14 @@ S:   Maintained
 F: drivers/gpio/gpio-ath79.c
 F: Documentation/devicetree/bindings/gpio/gpio-ath79.txt
 
+ATHEROS 71XX/9XXX USB PHY DRIVER
+M: Alban Bedel 
+W: https://github.com/AlbanBedel/linux
+T: git git://github.com/AlbanBedel/linux
+S: Maintained
+F: drivers/phy/qualcomm/phy-ath79-usb.c
+F: Documentation/devicetree/bindings/phy/phy-ath79-usb.txt
+
 ATHEROS ATH GENERIC UTILITIES
 M: "Luis R. Rodriguez" 
 L: linux-wirel...@vger.kernel.org
diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index 7bfa64b..632a0e7 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -1,6 +1,15 @@
 #
-# Phy drivers for Qualcomm platforms
+# Phy drivers for Qualcomm and Atheros platforms
 #
+config PHY_ATH79_USB
+   tristate "Atheros AR71XX/9XXX USB PHY driver"
+   depends on OF && (ATH79 || COMPILE_TEST)
+   default y if USB_EHCI_HCD_PLATFORM || USB_OHCI_HCD_PLATFORM
+   select RESET_CONTROLLER
+   select GENERIC_PHY
+   help
+ Enable this to support the USB PHY on Atheros AR71XX/9XXX SoCs.
+
 config PHY_QCOM_APQ8064_SATA
tristate "Qualcomm APQ8064 SATA SerDes/PHY driver"
depends on ARCH_QCOM
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index 9abb789..deb831f4 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_PHY_ATH79_USB)+= phy-ath79-usb.o
 obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)+= phy-qcom-apq8064-sata.o
 obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)+= phy-qcom-ipq806x-sata.o
 obj-$(CONFIG_PHY_QCOM_QMP) += phy-qcom-qmp.o
diff --git a/drivers/phy/qualcomm/phy-ath79-usb.c 
b/drivers/phy/qualcomm/phy-ath79-usb.c
new file mode 100644
index 000..6fd6e07
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-ath79-usb.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Atheros AR71XX/9XXX USB PHY driver
+ *
+ * Copyright (C) 2015-2018 Alban Bedel 
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+struct ath79_usb_phy {
+   struct reset_control *reset;
+   /* The suspend override logic is inverted, hence the no prefix
+* to make the code a bit easier to understand.
+*/
+   struct reset_control *no_suspend_override;
+};
+
+static int ath79_usb_phy_power_on(struct phy *phy)
+{
+   struct ath79_usb_phy *priv = phy_get_drvdata(phy);
+   int err = 0;
+
+   if (priv->no_suspend_override) {
+   err = reset_control_assert(priv->no_suspend_override);
+   if (err)
+   return err;
+   }
+
+   err = reset_control_deassert(priv->reset);
+   if (err && priv->no_suspend_override)
+   reset_control_assert(priv->no_suspend_override);
+
+   return err;
+}
+
+static int ath79_usb_phy_power_off(struct phy *phy)
+{
+   struct ath79_usb_phy *priv = phy_get_drvdata(phy);
+   int err = 0;
+
+   err = reset_control_assert(priv->reset);
+   if (err)
+   return err;
+
+   if (priv->no_suspend_override) {
+   err = reset_control_deassert(priv->no_suspend_override);
+   if (err)
+   reset_control_deassert(priv->reset);
+   }
+
+   return err;
+}
+
+static const struct phy_ops ath79_usb_phy_ops = {
+   .power_on   = ath79_usb_phy_power_on,
+   .power_off  = ath79_usb_phy_power_off,
+   .owner  = THIS_MODULE,
+};
+
+static int ath79_usb_phy_probe(struct platform_device *pdev)
+{
+   struct ath79_usb_phy *priv;
+   struct phy *phy;
+
+   priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
+   if (!priv)
+   return -ENOMEM;
+
+   priv->reset = devm_reset_control_get(>dev, "usb-phy");
+   if (IS_ERR(priv->reset))
+   return PTR_ERR(priv->reset);
+
+   priv->no_suspend_override = devm_reset_control_get_optional(
+   >dev, "usb-suspend-override");
+   if (IS_ERR(priv->no_suspend_override))
+   return PTR_ERR(priv->no_suspend_override);
+
+   phy = devm_phy_create(>dev, NULL, 

Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()

[Wolfram Sang]

> > -   info.archdata = _ad;
> 
> Why did you drop this?

If the removal is safe, it should be a seperate patch, I mean.



signature.asc
Description: PGP signature

Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()

[Wolfram Sang]

> > -   info.archdata = _ad;
> 
> Why did you drop this?

If the removal is safe, it should be a seperate patch, I mean.



signature.asc
Description: PGP signature

Re: efisubsys_init takes more than a few milliseconds

[Ard Biesheuvel]

Hello Paul,

On 24 March 2018 at 22:10, Paul Menzel  wrote:
> Dear Ard,
>
>
> According to `initcall_debug`, `efisubsys_init` takes more than a few
> milliseconds to execute on a Dell XPS 13 9370 (Intel(R) Core(TM) i7-8550U
> CPU @ 1.80GHz).
>
>> ```
>> […]
>> [ 0.144474] calling  efisubsys_init+0x0/0x2cf @ 1
>> [ 0.144474] Registered efivars operations
>> [ 0.173690] initcall efisubsys_init+0x0/0x2cf returned 0 after 27343 usecs
>> […]
>> ```
>
>
> To get a vanilla Linux kernel to boot in well under one second, it’d be nice
> if the time could be improved. Do you know, why it takes so long?
>
> According to `bootgraph.py` from pm-graph [1][2] it takes even a little
> longer.
>
>> efisubsys_init: start=690.841, end=720.493, length(w/o overhead)=31.250
>> ms, return=0
>
>
> There are several dozen calls to `virt_efi_get_next_variable()` all but one
> taking around 0.335 ms. This path needs to be optimized. Is that possible?
>

That depends. These are firmware calls, so to make these calls faster,
you need to modify the firmware, not the kernel.

We may be able to make more intrusive changes to get rid of this
delay, e.g., spin up a special kernel thread, but I'd have to check in
more detail. In the mean time, you can try passing 'efi=noruntime' to
the kernel.


> To reproduce this, clone the pm-graph repository [2], use `sudo
> ./bootgraph.py -f -fstat -maxdepth 10 -manual` to see what to add to
> `/boot/grub/grub.cfg`. Then reboot, and execute `sudo ./bootgraph.py -f
> -fstat -maxdepth 10`.
>
> If your system is powerful enough, you can use a higher maximum depth. I
> didn’t get around how `-cgfilter` works to get smaller HTML files.
>
>
> Kind regards,
>
> Paul
>
>
> [1] https://01.org/suspendresume
> [2] https://github.com/01org/pm-graph

Re: efisubsys_init takes more than a few milliseconds

[Ard Biesheuvel]

Hello Paul,

On 24 March 2018 at 22:10, Paul Menzel  wrote:
> Dear Ard,
>
>
> According to `initcall_debug`, `efisubsys_init` takes more than a few
> milliseconds to execute on a Dell XPS 13 9370 (Intel(R) Core(TM) i7-8550U
> CPU @ 1.80GHz).
>
>> ```
>> […]
>> [ 0.144474] calling  efisubsys_init+0x0/0x2cf @ 1
>> [ 0.144474] Registered efivars operations
>> [ 0.173690] initcall efisubsys_init+0x0/0x2cf returned 0 after 27343 usecs
>> […]
>> ```
>
>
> To get a vanilla Linux kernel to boot in well under one second, it’d be nice
> if the time could be improved. Do you know, why it takes so long?
>
> According to `bootgraph.py` from pm-graph [1][2] it takes even a little
> longer.
>
>> efisubsys_init: start=690.841, end=720.493, length(w/o overhead)=31.250
>> ms, return=0
>
>
> There are several dozen calls to `virt_efi_get_next_variable()` all but one
> taking around 0.335 ms. This path needs to be optimized. Is that possible?
>

That depends. These are firmware calls, so to make these calls faster,
you need to modify the firmware, not the kernel.

We may be able to make more intrusive changes to get rid of this
delay, e.g., spin up a special kernel thread, but I'd have to check in
more detail. In the mean time, you can try passing 'efi=noruntime' to
the kernel.


> To reproduce this, clone the pm-graph repository [2], use `sudo
> ./bootgraph.py -f -fstat -maxdepth 10 -manual` to see what to add to
> `/boot/grub/grub.cfg`. Then reboot, and execute `sudo ./bootgraph.py -f
> -fstat -maxdepth 10`.
>
> If your system is powerful enough, you can use a higher maximum depth. I
> didn’t get around how `-cgfilter` works to get smaller HTML files.
>
>
> Kind regards,
>
> Paul
>
>
> [1] https://01.org/suspendresume
> [2] https://github.com/01org/pm-graph

Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()

[Wolfram Sang]

Hi Boris,

> - rebase on v4.15-rc1

This code has changed a little meanwhile. Please check my for-next
branch. Some changes are identical, some similar.

> - info.archdata = _ad;

Why did you drop this?

Regards,

   Wolfram



signature.asc
Description: PGP signature

Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()

[Wolfram Sang]

Hi Boris,

> - rebase on v4.15-rc1

This code has changed a little meanwhile. Please check my for-next
branch. Some changes are identical, some similar.

> - info.archdata = _ad;

Why did you drop this?

Regards,

   Wolfram



signature.asc
Description: PGP signature

Re: [PATCH net-next v2 2/2] cxgb4: collect hardware dump in second kernel

[Thadeu Lima de Souza Cascardo]

On Sat, Mar 24, 2018 at 04:26:34PM +0530, Rahul Lakkireddy wrote:
> Register callback to collect hardware/firmware dumps in second kernel
> before hardware/firmware is initialized.  The dumps for each device
> will be available under /sys/kernel/crashdd/cxgb4/ directory in second
> kernel.
> 
> Signed-off-by: Rahul Lakkireddy 
> Signed-off-by: Ganesh Goudar 
> ---
> v2:
> - No Changes.
> 
> Changes since rfc v2:
> - Update comments and commit message for sysfs change.
> 
> rfc v2:
> - Updated dump registration to the new API in patch 1.
[...]
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c 
> b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> index e880be8e3c45..265cb026f868 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> @@ -5527,6 +5527,18 @@ static int init_one(struct pci_dev *pdev, const struct 
> pci_device_id *ent)
>   if (err)
>   goto out_free_adapter;
>  
> + if (is_kdump_kernel()) {
> + /* Collect hardware state and append to
> +  * /sys/kernel/crashdd/cxgb4/ directory
> +  */
> + err = cxgb4_cudbg_crashdd_add_dump(adapter);
> + if (err) {
> + dev_warn(adapter->pdev_dev,
> +  "Fail collecting crash device dump, err: %d. 
> Continuing\n",
> +  err);
> + err = 0;
> + }
> + }
>  

The problem I see with this approach is that you require that the driver
is built into the kdump kernel (or present as a module in the kdump
initramfs), and that you will probe the device during the collection of
the dumps.

IMHO, if you are going to require the device to be probed by the same
driver during kdump, you might just as well use the device object itself
to present the crash data. I think that's what Stephen Hemminger meant
when he said to use sysfs. No need at all for any special crashdd. Just
add an attribute or attribute group to the device object.

Otherwise, as Eric Biederman pointed out, you should just add that data
into the vmcore before you kexec, so you don't even need to look at a
different file, and the driver does not even need to be present in the
kdump kernel.

Cascardo.

>   if (!is_t4(adapter->params.chip)) {
>   s_qpp = (QUEUESPERPAGEPF0_S +
> -- 
> 2.14.1

Re: [PATCH net-next v2 2/2] cxgb4: collect hardware dump in second kernel

[Thadeu Lima de Souza Cascardo]

On Sat, Mar 24, 2018 at 04:26:34PM +0530, Rahul Lakkireddy wrote:
> Register callback to collect hardware/firmware dumps in second kernel
> before hardware/firmware is initialized.  The dumps for each device
> will be available under /sys/kernel/crashdd/cxgb4/ directory in second
> kernel.
> 
> Signed-off-by: Rahul Lakkireddy 
> Signed-off-by: Ganesh Goudar 
> ---
> v2:
> - No Changes.
> 
> Changes since rfc v2:
> - Update comments and commit message for sysfs change.
> 
> rfc v2:
> - Updated dump registration to the new API in patch 1.
[...]
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c 
> b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> index e880be8e3c45..265cb026f868 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> @@ -5527,6 +5527,18 @@ static int init_one(struct pci_dev *pdev, const struct 
> pci_device_id *ent)
>   if (err)
>   goto out_free_adapter;
>  
> + if (is_kdump_kernel()) {
> + /* Collect hardware state and append to
> +  * /sys/kernel/crashdd/cxgb4/ directory
> +  */
> + err = cxgb4_cudbg_crashdd_add_dump(adapter);
> + if (err) {
> + dev_warn(adapter->pdev_dev,
> +  "Fail collecting crash device dump, err: %d. 
> Continuing\n",
> +  err);
> + err = 0;
> + }
> + }
>  

The problem I see with this approach is that you require that the driver
is built into the kdump kernel (or present as a module in the kdump
initramfs), and that you will probe the device during the collection of
the dumps.

IMHO, if you are going to require the device to be probed by the same
driver during kdump, you might just as well use the device object itself
to present the crash data. I think that's what Stephen Hemminger meant
when he said to use sysfs. No need at all for any special crashdd. Just
add an attribute or attribute group to the device object.

Otherwise, as Eric Biederman pointed out, you should just add that data
into the vmcore before you kexec, so you don't even need to look at a
different file, and the driver does not even need to be present in the
kdump kernel.

Cascardo.

>   if (!is_t4(adapter->params.chip)) {
>   s_qpp = (QUEUESPERPAGEPF0_S +
> -- 
> 2.14.1

efisubsys_init takes more than a few milliseconds

[Paul Menzel]

Dear Ard,


According to `initcall_debug`, `efisubsys_init` takes more than a few 
milliseconds to execute on a Dell XPS 13 9370 (Intel(R) Core(TM) 
i7-8550U CPU @ 1.80GHz).



```
[…]
[0.144474] calling  efisubsys_init+0x0/0x2cf @ 1
[0.144474] Registered efivars operations
[0.173690] initcall efisubsys_init+0x0/0x2cf returned 0 after 27343 usecs
[…]
```


To get a vanilla Linux kernel to boot in well under one second, it’d be 
nice if the time could be improved. Do you know, why it takes so long?


According to `bootgraph.py` from pm-graph [1][2] it takes even a little 
longer.



efisubsys_init: start=690.841, end=720.493, length(w/o overhead)=31.250 ms, 
return=0


There are several dozen calls to `virt_efi_get_next_variable()` all but 
one taking around 0.335 ms. This path needs to be optimized. Is that 
possible?


To reproduce this, clone the pm-graph repository [2], use `sudo 
./bootgraph.py -f -fstat -maxdepth 10 -manual` to see what to add to 
`/boot/grub/grub.cfg`. Then reboot, and execute `sudo ./bootgraph.py -f 
-fstat -maxdepth 10`.


If your system is powerful enough, you can use a higher maximum depth. I 
didn’t get around how `-cgfilter` works to get smaller HTML files.



Kind regards,

Paul


[1] https://01.org/suspendresume
[2] https://github.com/01org/pm-graph

efisubsys_init takes more than a few milliseconds

[Paul Menzel]

Dear Ard,


According to `initcall_debug`, `efisubsys_init` takes more than a few 
milliseconds to execute on a Dell XPS 13 9370 (Intel(R) Core(TM) 
i7-8550U CPU @ 1.80GHz).



```
[…]
[0.144474] calling  efisubsys_init+0x0/0x2cf @ 1
[0.144474] Registered efivars operations
[0.173690] initcall efisubsys_init+0x0/0x2cf returned 0 after 27343 usecs
[…]
```


To get a vanilla Linux kernel to boot in well under one second, it’d be 
nice if the time could be improved. Do you know, why it takes so long?


According to `bootgraph.py` from pm-graph [1][2] it takes even a little 
longer.



efisubsys_init: start=690.841, end=720.493, length(w/o overhead)=31.250 ms, 
return=0


There are several dozen calls to `virt_efi_get_next_variable()` all but 
one taking around 0.335 ms. This path needs to be optimized. Is that 
possible?


To reproduce this, clone the pm-graph repository [2], use `sudo 
./bootgraph.py -f -fstat -maxdepth 10 -manual` to see what to add to 
`/boot/grub/grub.cfg`. Then reboot, and execute `sudo ./bootgraph.py -f 
-fstat -maxdepth 10`.


If your system is powerful enough, you can use a higher maximum depth. I 
didn’t get around how `-cgfilter` works to get smaller HTML files.



Kind regards,

Paul


[1] https://01.org/suspendresume
[2] https://github.com/01org/pm-graph

[PATCH] staging: pi433: cleanup tx_fifo locking

[Valentin Vidic]

pi433_write requires locking due to multiple kfifo writers.  After
acquiring the lock check if enough free space is available in the kfifo
to write the whole message. This check should prevent partial writes to
kfifo so kfifo_reset is not needed anymore.

pi433_tx_thread is the only kfifo reader so it does not require locking
after kfifo_reset is also removed.

Signed-off-by: Valentin Vidic 
---
 drivers/staging/pi433/pi433_if.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index d1e0ddbc79ce..97239b0eb322 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -87,7 +87,7 @@ struct pi433_device {
 
/* tx related values */
STRUCT_KFIFO_REC_1(MSG_FIFO_SIZE) tx_fifo;
-   struct mutextx_fifo_lock; // TODO: check, whether necessary 
or obsolete
+   struct mutextx_fifo_lock; /* serialize multiple writers */
struct task_struct  *tx_task_struct;
wait_queue_head_t   tx_wait_queue;
u8  free_in_fifo;
@@ -589,19 +589,15 @@ pi433_tx_thread(void *data)
 * - size of message
 * - message
 */
-   mutex_lock(>tx_fifo_lock);
-
retval = kfifo_out(>tx_fifo, _cfg, sizeof(tx_cfg));
if (retval != sizeof(tx_cfg)) {
dev_dbg(device->dev, "reading tx_cfg from fifo failed: 
got %d byte(s), expected %d", retval, (unsigned int)sizeof(tx_cfg));
-   mutex_unlock(>tx_fifo_lock);
continue;
}
 
retval = kfifo_out(>tx_fifo, , sizeof(size_t));
if (retval != sizeof(size_t)) {
dev_dbg(device->dev, "reading msg size from fifo 
failed: got %d, expected %d", retval, (unsigned int)sizeof(size_t));
-   mutex_unlock(>tx_fifo_lock);
continue;
}
 
@@ -634,7 +630,6 @@ pi433_tx_thread(void *data)
   sizeof(device->buffer) - position);
dev_dbg(device->dev,
"read %d message byte(s) from fifo queue.", retval);
-   mutex_unlock(>tx_fifo_lock);
 
/* if rx is active, we need to interrupt the waiting for
 * incoming telegrams, to be able to send something.
@@ -818,7 +813,7 @@ pi433_write(struct file *filp, const char __user *buf,
struct pi433_instance   *instance;
struct pi433_device *device;
int retval;
-   unsigned intcopied;
+   unsigned intrequired, available, copied;
 
instance = filp->private_data;
device = instance->device;
@@ -833,6 +828,16 @@ pi433_write(struct file *filp, const char __user *buf,
 * - message
 */
mutex_lock(>tx_fifo_lock);
+
+   required = sizeof(instance->tx_cfg) + sizeof(size_t) + count;
+   available = kfifo_avail(>tx_fifo);
+   if (required > available) {
+   dev_dbg(device->dev, "write to fifo failed: %d bytes required 
but %d available",
+   required, available);
+   mutex_unlock(>tx_fifo_lock);
+   return -EAGAIN;
+   }
+
retval = kfifo_in(>tx_fifo, >tx_cfg,
  sizeof(instance->tx_cfg));
if (retval != sizeof(instance->tx_cfg))
@@ -856,7 +861,6 @@ pi433_write(struct file *filp, const char __user *buf,
 
 abort:
dev_dbg(device->dev, "write to fifo failed: 0x%x", retval);
-   kfifo_reset(>tx_fifo); // TODO: maybe find a solution, not to 
discard already stored, valid entries
mutex_unlock(>tx_fifo_lock);
return -EAGAIN;
 }
-- 
2.16.2

[PATCH] staging: pi433: cleanup tx_fifo locking

[Valentin Vidic]

pi433_write requires locking due to multiple kfifo writers.  After
acquiring the lock check if enough free space is available in the kfifo
to write the whole message. This check should prevent partial writes to
kfifo so kfifo_reset is not needed anymore.

pi433_tx_thread is the only kfifo reader so it does not require locking
after kfifo_reset is also removed.

Signed-off-by: Valentin Vidic 
---
 drivers/staging/pi433/pi433_if.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index d1e0ddbc79ce..97239b0eb322 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -87,7 +87,7 @@ struct pi433_device {
 
/* tx related values */
STRUCT_KFIFO_REC_1(MSG_FIFO_SIZE) tx_fifo;
-   struct mutextx_fifo_lock; // TODO: check, whether necessary 
or obsolete
+   struct mutextx_fifo_lock; /* serialize multiple writers */
struct task_struct  *tx_task_struct;
wait_queue_head_t   tx_wait_queue;
u8  free_in_fifo;
@@ -589,19 +589,15 @@ pi433_tx_thread(void *data)
 * - size of message
 * - message
 */
-   mutex_lock(>tx_fifo_lock);
-
retval = kfifo_out(>tx_fifo, _cfg, sizeof(tx_cfg));
if (retval != sizeof(tx_cfg)) {
dev_dbg(device->dev, "reading tx_cfg from fifo failed: 
got %d byte(s), expected %d", retval, (unsigned int)sizeof(tx_cfg));
-   mutex_unlock(>tx_fifo_lock);
continue;
}
 
retval = kfifo_out(>tx_fifo, , sizeof(size_t));
if (retval != sizeof(size_t)) {
dev_dbg(device->dev, "reading msg size from fifo 
failed: got %d, expected %d", retval, (unsigned int)sizeof(size_t));
-   mutex_unlock(>tx_fifo_lock);
continue;
}
 
@@ -634,7 +630,6 @@ pi433_tx_thread(void *data)
   sizeof(device->buffer) - position);
dev_dbg(device->dev,
"read %d message byte(s) from fifo queue.", retval);
-   mutex_unlock(>tx_fifo_lock);
 
/* if rx is active, we need to interrupt the waiting for
 * incoming telegrams, to be able to send something.
@@ -818,7 +813,7 @@ pi433_write(struct file *filp, const char __user *buf,
struct pi433_instance   *instance;
struct pi433_device *device;
int retval;
-   unsigned intcopied;
+   unsigned intrequired, available, copied;
 
instance = filp->private_data;
device = instance->device;
@@ -833,6 +828,16 @@ pi433_write(struct file *filp, const char __user *buf,
 * - message
 */
mutex_lock(>tx_fifo_lock);
+
+   required = sizeof(instance->tx_cfg) + sizeof(size_t) + count;
+   available = kfifo_avail(>tx_fifo);
+   if (required > available) {
+   dev_dbg(device->dev, "write to fifo failed: %d bytes required 
but %d available",
+   required, available);
+   mutex_unlock(>tx_fifo_lock);
+   return -EAGAIN;
+   }
+
retval = kfifo_in(>tx_fifo, >tx_cfg,
  sizeof(instance->tx_cfg));
if (retval != sizeof(instance->tx_cfg))
@@ -856,7 +861,6 @@ pi433_write(struct file *filp, const char __user *buf,
 
 abort:
dev_dbg(device->dev, "write to fifo failed: 0x%x", retval);
-   kfifo_reset(>tx_fifo); // TODO: maybe find a solution, not to 
discard already stored, valid entries
mutex_unlock(>tx_fifo_lock);
return -EAGAIN;
 }
-- 
2.16.2

Re: [PATCH] ftrace: fix stddev calculation in function profiler (again)

[Matthias Schiffer]

On 03/24/2018 05:26 PM, Matthias Schiffer wrote:
> It is well-known that it is not possible to accurately calculate variances
> just by accumulating squared samples; in fact, such an approach can even
> result in negative numbers. An earlier attempt to fix the calculation
> referred to Welford's method, but did not implement it correctly, leading
> to meeaningless output like the following:
> 
>   nf_conntrack_proto_fini 50373.523 us7.470 us3234315951 us
> 
> Welford's method uses one do_div() in the tracing path; this cannot be
> avoided. The average time is added to struct ftrace_profile, so only a
> single division is required. I also considered the following alternatives:
> 
> 1) Only keeping the avg field and removing the aggregated time would
> greatly lead to severe rounding errors in calculating the total time based
> on counter and avg.
> 
> 2) Calculating both the old and the new average in profile_graph_return()
> instead of storing it in struct ftrace_profile would require a second
> division.
> 
> 3) I managed to transform Welford's equations in a way that uses the total
> time instead of the average and requires only a single division.
> Unfortunately, the divisor is counter^3 in this case, easily overflowing
> even 64bit integers.
> 
> Ruling out the above alternatives, I chose the present approach to fix the
> issue.
> 
> Fixes: e330b3bcd831 ("tracing: Show sample std dev in function profiling")
> Fixes: 52d85d763086 ("ftrace: Fix stddev calculation in function profiler")
> Signed-off-by: Matthias Schiffer 


Hmm, in further testing, I'm seeing profile_graph_return() being called
without rec->counter getting incremented a lot, completely throwing off the
stddev calculation (and also slightly displacing the average). Is this
expected?

Should we possibly move the counter increment to profile_graph_return() in
the CONFIG_FUNCTION_GRAPH_TRACER case, or introduce a second counter for
returns?

Matthias



> ---
>  kernel/trace/ftrace.c | 33 +++--
>  1 file changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index eac9ce2c57a2..16dce67b855a 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -460,7 +460,8 @@ struct ftrace_profile {
>   unsigned long   counter;
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>   unsigned long long  time;
> - unsigned long long  time_squared;
> + unsigned long long  avg;
> + unsigned long long  stddev;
>  #endif
>  };
>  
> @@ -580,7 +581,6 @@ static int function_stat_show(struct seq_file *m, void *v)
>   int ret = 0;
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>   static struct trace_seq s;
> - unsigned long long avg;
>   unsigned long long stddev;
>  #endif
>   mutex_lock(_profile_lock);
> @@ -592,9 +592,7 @@ static int function_stat_show(struct seq_file *m, void *v)
>   }
>  
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> - avg = rec->time;
> - do_div(avg, rec->counter);
> - if (tracing_thresh && (avg < tracing_thresh))
> + if (tracing_thresh && (rec->avg < tracing_thresh))
>   goto out;
>  #endif
>  
> @@ -608,24 +606,19 @@ static int function_stat_show(struct seq_file *m, void 
> *v)
>   if (rec->counter <= 1)
>   stddev = 0;
>   else {
> - /*
> -  * Apply Welford's method:
> -  * s^2 = 1 / (n * (n-1)) * (n * \Sum (x_i)^2 - (\Sum x_i)^2)
> -  */
> - stddev = rec->counter * rec->time_squared -
> -  rec->time * rec->time;
> + stddev = rec->stddev;
>  
>   /*
>* Divide only 1000 for ns^2 -> us^2 conversion.
>* trace_print_graph_duration will divide 1000 again.
>*/
> - do_div(stddev, rec->counter * (rec->counter - 1) * 1000);
> + do_div(stddev, 1000 * (rec->counter - 1));
>   }
>  
>   trace_seq_init();
>   trace_print_graph_duration(rec->time, );
>   trace_seq_puts(, "");
> - trace_print_graph_duration(avg, );
> + trace_print_graph_duration(rec->avg, );
>   trace_seq_puts(, "");
>   trace_print_graph_duration(stddev, );
>   trace_print_seq(m, );
> @@ -905,8 +898,20 @@ static void profile_graph_return(struct ftrace_graph_ret 
> *trace)
>  
>   rec = ftrace_find_profiled_func(stat, trace->func);
>   if (rec) {
> + unsigned long long avg, delta1, delta2;
> +
>   rec->time += calltime;
> - rec->time_squared += calltime * calltime;
> +
> + /* Apply Welford's method */
> + delta1 = calltime - rec->avg;
> +
> + avg = rec->time;
> + do_div(avg, rec->counter);
> + rec->avg = avg;
> +
> + delta2 = calltime - rec->avg;
> +
> + rec->stddev += 

Re: [PATCH] ftrace: fix stddev calculation in function profiler (again)

[Matthias Schiffer]

On 03/24/2018 05:26 PM, Matthias Schiffer wrote:
> It is well-known that it is not possible to accurately calculate variances
> just by accumulating squared samples; in fact, such an approach can even
> result in negative numbers. An earlier attempt to fix the calculation
> referred to Welford's method, but did not implement it correctly, leading
> to meeaningless output like the following:
> 
>   nf_conntrack_proto_fini 50373.523 us7.470 us3234315951 us
> 
> Welford's method uses one do_div() in the tracing path; this cannot be
> avoided. The average time is added to struct ftrace_profile, so only a
> single division is required. I also considered the following alternatives:
> 
> 1) Only keeping the avg field and removing the aggregated time would
> greatly lead to severe rounding errors in calculating the total time based
> on counter and avg.
> 
> 2) Calculating both the old and the new average in profile_graph_return()
> instead of storing it in struct ftrace_profile would require a second
> division.
> 
> 3) I managed to transform Welford's equations in a way that uses the total
> time instead of the average and requires only a single division.
> Unfortunately, the divisor is counter^3 in this case, easily overflowing
> even 64bit integers.
> 
> Ruling out the above alternatives, I chose the present approach to fix the
> issue.
> 
> Fixes: e330b3bcd831 ("tracing: Show sample std dev in function profiling")
> Fixes: 52d85d763086 ("ftrace: Fix stddev calculation in function profiler")
> Signed-off-by: Matthias Schiffer 


Hmm, in further testing, I'm seeing profile_graph_return() being called
without rec->counter getting incremented a lot, completely throwing off the
stddev calculation (and also slightly displacing the average). Is this
expected?

Should we possibly move the counter increment to profile_graph_return() in
the CONFIG_FUNCTION_GRAPH_TRACER case, or introduce a second counter for
returns?

Matthias



> ---
>  kernel/trace/ftrace.c | 33 +++--
>  1 file changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index eac9ce2c57a2..16dce67b855a 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -460,7 +460,8 @@ struct ftrace_profile {
>   unsigned long   counter;
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>   unsigned long long  time;
> - unsigned long long  time_squared;
> + unsigned long long  avg;
> + unsigned long long  stddev;
>  #endif
>  };
>  
> @@ -580,7 +581,6 @@ static int function_stat_show(struct seq_file *m, void *v)
>   int ret = 0;
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>   static struct trace_seq s;
> - unsigned long long avg;
>   unsigned long long stddev;
>  #endif
>   mutex_lock(_profile_lock);
> @@ -592,9 +592,7 @@ static int function_stat_show(struct seq_file *m, void *v)
>   }
>  
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> - avg = rec->time;
> - do_div(avg, rec->counter);
> - if (tracing_thresh && (avg < tracing_thresh))
> + if (tracing_thresh && (rec->avg < tracing_thresh))
>   goto out;
>  #endif
>  
> @@ -608,24 +606,19 @@ static int function_stat_show(struct seq_file *m, void 
> *v)
>   if (rec->counter <= 1)
>   stddev = 0;
>   else {
> - /*
> -  * Apply Welford's method:
> -  * s^2 = 1 / (n * (n-1)) * (n * \Sum (x_i)^2 - (\Sum x_i)^2)
> -  */
> - stddev = rec->counter * rec->time_squared -
> -  rec->time * rec->time;
> + stddev = rec->stddev;
>  
>   /*
>* Divide only 1000 for ns^2 -> us^2 conversion.
>* trace_print_graph_duration will divide 1000 again.
>*/
> - do_div(stddev, rec->counter * (rec->counter - 1) * 1000);
> + do_div(stddev, 1000 * (rec->counter - 1));
>   }
>  
>   trace_seq_init();
>   trace_print_graph_duration(rec->time, );
>   trace_seq_puts(, "");
> - trace_print_graph_duration(avg, );
> + trace_print_graph_duration(rec->avg, );
>   trace_seq_puts(, "");
>   trace_print_graph_duration(stddev, );
>   trace_print_seq(m, );
> @@ -905,8 +898,20 @@ static void profile_graph_return(struct ftrace_graph_ret 
> *trace)
>  
>   rec = ftrace_find_profiled_func(stat, trace->func);
>   if (rec) {
> + unsigned long long avg, delta1, delta2;
> +
>   rec->time += calltime;
> - rec->time_squared += calltime * calltime;
> +
> + /* Apply Welford's method */
> + delta1 = calltime - rec->avg;
> +
> + avg = rec->time;
> + do_div(avg, rec->counter);
> + rec->avg = avg;
> +
> + delta2 = calltime - rec->avg;
> +
> + rec->stddev += delta1 * delta2;
>   }
>  
>   

Re: [PATCH 1/2] fs: Extend mount_ns with support for a fast namespace to vfsmount function

[Al Viro]

On Sat, Mar 24, 2018 at 11:12:02AM -0500, Eric W. Biederman wrote:

> > This is completely wrong.  Look:
> > * SB_KERNMOUNT and !SB_KERNMOUNT cases are almost entirely isolated;
> > completely so once that ns_to_mnt becomes unconditionally non-NULL.  
> > * in !SB_KERNMOUNT passing ns_to_mnt() is pointless - you might as
> > well pass existing vfsmount (or ERR_PTR()) and use _that_.  fill_super()
> > is not used at all in that case.
> > * is SB_KERNMOUNT ns_to_mnt serves only as a flag, eventually
> > constant true.
> >
> > So let's split it in two helpers and give them sane arguments.
> 
> Everything I look at with multiple helpers feels even worse to me.
> The above has the advantage it is the minimal change to fix the
> regression.  So I am not worried about code correctness.

> I keep wondering is the intention long term to fix sget so it has an
> efficient data structure for finding super blocks (like an rbtree) or if
> the intention is to deprecate sget entirely and just have everything
> call alloc_super, and be responsible for their own data structures for
> finding existing superblocks.
>
> At this point since we are not in agreement on a proper fix I am going
> to plan on just queueing up a revert.   So that we don't ship 4.16 with
> a regression in a permission check.

Permission check is trivial to put back in; I'll do that.

FWIW, I don't believe that sget_userns() is a good place for any kind of
universal permission checks.  It's a library helper, not a place everything
must come through when mounting something.  So's mount_ns(), etc.

BTW, will you be at LSF?  I would suggest discussing the architectural
issues there - they are directly related to fsmount() proposals...

Re: [PATCH 1/2] fs: Extend mount_ns with support for a fast namespace to vfsmount function

[Al Viro]

On Sat, Mar 24, 2018 at 11:12:02AM -0500, Eric W. Biederman wrote:

> > This is completely wrong.  Look:
> > * SB_KERNMOUNT and !SB_KERNMOUNT cases are almost entirely isolated;
> > completely so once that ns_to_mnt becomes unconditionally non-NULL.  
> > * in !SB_KERNMOUNT passing ns_to_mnt() is pointless - you might as
> > well pass existing vfsmount (or ERR_PTR()) and use _that_.  fill_super()
> > is not used at all in that case.
> > * is SB_KERNMOUNT ns_to_mnt serves only as a flag, eventually
> > constant true.
> >
> > So let's split it in two helpers and give them sane arguments.
> 
> Everything I look at with multiple helpers feels even worse to me.
> The above has the advantage it is the minimal change to fix the
> regression.  So I am not worried about code correctness.

> I keep wondering is the intention long term to fix sget so it has an
> efficient data structure for finding super blocks (like an rbtree) or if
> the intention is to deprecate sget entirely and just have everything
> call alloc_super, and be responsible for their own data structures for
> finding existing superblocks.
>
> At this point since we are not in agreement on a proper fix I am going
> to plan on just queueing up a revert.   So that we don't ship 4.16 with
> a regression in a permission check.

Permission check is trivial to put back in; I'll do that.

FWIW, I don't believe that sget_userns() is a good place for any kind of
universal permission checks.  It's a library helper, not a place everything
must come through when mounting something.  So's mount_ns(), etc.

BTW, will you be at LSF?  I would suggest discussing the architectural
issues there - they are directly related to fsmount() proposals...

**Herzlichen Glückwunsch**

[Euro Millions]

Herzlichen Glückwunsch, Sie haben 650.000 Euro in den monatlichen Gewinnspielen 
von Euro Millions/Google Promo am 10. März 2018 gewonnen.

Kontaktieren Sie unseren Schadenregulierungsbeauftragten mit den folgenden 
Informationen

Vollständiger Name
Heimatadresse
Geschlecht
Alter
Telefon

Mr.Pianese Germano

**Herzlichen Glückwunsch**

[Euro Millions]

Herzlichen Glückwunsch, Sie haben 650.000 Euro in den monatlichen Gewinnspielen 
von Euro Millions/Google Promo am 10. März 2018 gewonnen.

Kontaktieren Sie unseren Schadenregulierungsbeauftragten mit den folgenden 
Informationen

Vollständiger Name
Heimatadresse
Geschlecht
Alter
Telefon

Mr.Pianese Germano

[PATCH] x86/purgatory: Remove -MD from KBUILD_CFLAGS

[Sven Wegener]

The kernel build system already takes care of generating the dependency
files. Having the additional -MD in KBUILD_CFLAGS leads to stray
..d files in the build directory when we call the cc-option macro.

Signed-off-by: Sven Wegener 
---
 arch/x86/purgatory/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 2f15a2ac4209..d70c15de417b 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -16,7 +16,7 @@ KCOV_INSTRUMENT := n
 # in turn leaves some undefined symbols like __fentry__ in purgatory and not
 # sure how to relocate those. Like kexec-tools, use custom flags.
 
-KBUILD_CFLAGS := -fno-strict-aliasing -Wall -Wstrict-prototypes 
-fno-zero-initialized-in-bss -fno-builtin -ffreestanding -c -MD -Os 
-mcmodel=large
+KBUILD_CFLAGS := -fno-strict-aliasing -Wall -Wstrict-prototypes 
-fno-zero-initialized-in-bss -fno-builtin -ffreestanding -c -Os -mcmodel=large
 KBUILD_CFLAGS += -m$(BITS)
 KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
 

[PATCH] x86/purgatory: Remove -MD from KBUILD_CFLAGS

[Sven Wegener]

The kernel build system already takes care of generating the dependency
files. Having the additional -MD in KBUILD_CFLAGS leads to stray
..d files in the build directory when we call the cc-option macro.

Signed-off-by: Sven Wegener 
---
 arch/x86/purgatory/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 2f15a2ac4209..d70c15de417b 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -16,7 +16,7 @@ KCOV_INSTRUMENT := n
 # in turn leaves some undefined symbols like __fentry__ in purgatory and not
 # sure how to relocate those. Like kexec-tools, use custom flags.
 
-KBUILD_CFLAGS := -fno-strict-aliasing -Wall -Wstrict-prototypes 
-fno-zero-initialized-in-bss -fno-builtin -ffreestanding -c -MD -Os 
-mcmodel=large
+KBUILD_CFLAGS := -fno-strict-aliasing -Wall -Wstrict-prototypes 
-fno-zero-initialized-in-bss -fno-builtin -ffreestanding -c -Os -mcmodel=large
 KBUILD_CFLAGS += -m$(BITS)
 KBUILD_CFLAGS += $(call cc-option,-fno-PIE)