Re: [PATCH v2] nfit: add Hyper-V NVDIMM DSM command set to white list
On Fri, Feb 1, 2019 at 7:32 PM Dexuan Cui wrote: > > > From: Dan Williams > > Sent: Friday, February 1, 2019 5:29 PM > > ... > > Honestly, the quickest path to something functional for Linux is to > > simply delete the _LSR support and use raw mode defined namespaces. > > Why have labels if they are read-only and the region is sufficient for > > defining boundaries? > Hyper-V Virtual NVDIMM can already work with Ubuntu 19.04 virtual machine > running on Hyper-V, i.e. I can create a raw or fsdax namesapce, and create a > xfs or ext4 file sysetm in /dev/pmem0p1, and mount the file system with and > without "-o dax". The basic functionality is good. Only in label-less mode apparently. > My recent work is mainly for ndctl support, i.e. get the health info by ndctl, > and use ndctl to monitor the error events (if applicable). Right, but the recent fixes have exposed Linux to a labelled namespace implementation that violates some of the driver's basic assumptions. To preserve the level of operation you're currently seeing Linux might need to add a hyper-v-specific quirk to force label-less operation. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] nfit: add Hyper-V NVDIMM DSM command set to white list
On Fri, Feb 1, 2019 at 6:17 PM Dexuan Cui wrote: > > From: Dan Williams [..] > > Honestly, the quickest path to something functional for Linux is to > > simply delete the _LSR support and use raw mode defined namespaces. > > Why have labels if they are read-only and the region is sufficient for > > defining boundaries? > > Just now Hyper-V team confirmed _LSW is not supported. > > But with Ubuntu 19.04 kernel (4.18.0-11-generic), I'm able to run the commands > without any issue: This is because Ubuntu is running in "label-less" mode. So all the writes it is performing for namespace reconfiguration are just going to the data-space of the region, not the labels. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC net-next 11/13] nfp: Handle SWITCHDEV_PORT_ATTR_GET event
Le 2/1/19 à 7:45 PM, Jakub Kicinski a écrit : > On Fri, 1 Feb 2019 14:06:55 -0800, Florian Fainelli wrote: >> Following patches will change the way we communicate getting or setting >> a port's attribute and use a blocking notifier to perform those tasks. >> >> Prepare nfp to support receiving notifier events targeting >> SWITCHDEV_PORT_ATTR_GET and simply translate that into the existing >> switchdev_ops::switchdev_port_attr_get operation. >> >> We register a single blocking switchdev notifier for the entire driver >> instance and we differentiate a "net" from a "repr" by comparing the >> network device's netdev_ops with the ones that this driver manages. >> >> Signed-off-by: Florian Fainelli > > Thanks Florian, the code looks good, only nit I have is - could you > move nfp_switchdev_blocking_event() to nfp_port.c and nfp_port.h? > We shouldn't touch nfp_net_common.c here. Sounds good, thanks for the suggestion. > > In general calling a notifier to get the parent_id (which is the only > thing all these SR-IOV NIC drivers implement) seems a tad heavy. It's > an immutable, read-only attribute of a port, perhaps we can break it > out? Could we make it an NDO, perhaps? A NDO would be fine with me, Ido, what do you think? > > That's just my knee jerk reaction, given that NIC drivers don't > implement any of the bridging side of switchdev I may not have a full > appreciation of the abstraction you are building here :) > Ido convinced me to convert the switchdev_port_attr_set() into a blocking notifier such that we could veto operations in switch drivers, once you do that, leaving the getter as switchdev_ops became pointless :) but yes, it's a lot of code just to get there. -- Florian ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC net-next 11/13] nfp: Handle SWITCHDEV_PORT_ATTR_GET event
On Fri, 1 Feb 2019 14:06:55 -0800, Florian Fainelli wrote: > Following patches will change the way we communicate getting or setting > a port's attribute and use a blocking notifier to perform those tasks. > > Prepare nfp to support receiving notifier events targeting > SWITCHDEV_PORT_ATTR_GET and simply translate that into the existing > switchdev_ops::switchdev_port_attr_get operation. > > We register a single blocking switchdev notifier for the entire driver > instance and we differentiate a "net" from a "repr" by comparing the > network device's netdev_ops with the ones that this driver manages. > > Signed-off-by: Florian Fainelli Thanks Florian, the code looks good, only nit I have is - could you move nfp_switchdev_blocking_event() to nfp_port.c and nfp_port.h? We shouldn't touch nfp_net_common.c here. In general calling a notifier to get the parent_id (which is the only thing all these SR-IOV NIC drivers implement) seems a tad heavy. It's an immutable, read-only attribute of a port, perhaps we can break it out? Could we make it an NDO, perhaps? That's just my knee jerk reaction, given that NIC drivers don't implement any of the bridging side of switchdev I may not have a full appreciation of the abstraction you are building here :) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2] nfit: add Hyper-V NVDIMM DSM command set to white list
> From: Dan Williams > Sent: Friday, February 1, 2019 5:29 PM > ... > Honestly, the quickest path to something functional for Linux is to > simply delete the _LSR support and use raw mode defined namespaces. > Why have labels if they are read-only and the region is sufficient for > defining boundaries? Hyper-V Virtual NVDIMM can already work with Ubuntu 19.04 virtual machine running on Hyper-V, i.e. I can create a raw or fsdax namesapce, and create a xfs or ext4 file sysetm in /dev/pmem0p1, and mount the file system with and without "-o dax". The basic functionality is good. My recent work is mainly for ndctl support, i.e. get the health info by ndctl, and use ndctl to monitor the error events (if applicable). Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2] nfit: add Hyper-V NVDIMM DSM command set to white list
> From: Dan Williams
> Sent: Friday, February 1, 2019 5:29 PM
> > ...
> > The "size" and "mode" still don't look right, but the improvement is that
> > now I can see a good descriptive "name", which I suppose is retrieved
> > from Hyper-V.
>
> Mode is right, there is no way for Hyper-V to create Linux fsdax mode
> namespaces it requires some setup using variables only Linux knows.
> Can you send the output of:
>
> ndctl read-labels -j all
The output is from a kernel built with the libnvdimm-pending branch plus
the one-line patch (label->flags &= ~NSLABEL_FLAG_LOCAL) in
init_active_labels():
root@decui-gen2-u1904:~# ndctl read-labels -j all
[
{
"dev":"nmem1",
"index":[
{
"signature":"NAMESPACE_INDEX",
"major":1,
"minor":2,
"labelsize":256,
"seq":1,
"nslot":2
},
{
"signature":"NAMESPACE_INDEX",
"major":1,
"minor":2,
"labelsize":256,
"seq":2,
"nslot":2
}
],
"label":[
{
"uuid":"c258aaab-f72b-e546-bfa5-be5e07761dbc",
"name":"Microsoft Hyper-V NVDIMM 1 Label",
"slot":0,
"position":0,
"nlabel":1,
"isetcookie":708891662257476870,
"lbasize":0,
"dpa":0,
"rawsize":137438953472,
"type_guid":"79d3f066-f3b4-7440-ac43-0d3318b78cdb",
"abstraction_guid":"----"
}
]
},
{
"dev":"nmem0",
"index":[
{
"signature":"NAMESPACE_INDEX",
"major":1,
"minor":2,
"labelsize":256,
"seq":1,
"nslot":2
},
{
"signature":"NAMESPACE_INDEX",
"major":1,
"minor":2,
"labelsize":256,
"seq":2,
"nslot":2
}
],
"label":[
{
"uuid":"9f0497a7-4453-7c40-ad35-21a791e00345",
"name":"Microsoft Hyper-V NVDIMM 0 Label",
"slot":0,
"position":0,
"nlabel":1,
"isetcookie":708891619307803909,
"lbasize":0,
"dpa":0,
"rawsize":34359738368,
"type_guid":"79d3f066-f3b4-7440-ac43-0d3318b78cdb",
"abstraction_guid":"----"
}
]
}
]
read 2 nmems
> > With Ubuntu 19.04 (4.18.0-11-generic), I get this:
> > (Note: the "mode" and "size" are correct. The "uuid" is different from
> > the above "9f0497a7-4453-7c40-ad35-21a791e00345" -- this is weird.)
> >
> > root@decui-gen2-u1904:~# ndctl list
> > [
> > {
> > "dev":"namespace1.0",
> > "mode":"raw",
> > "size":137438953472,
> > "blockdev":"pmem1"
> > },
> > {
> > "dev":"namespace0.0",
> > "mode":"fsdax",
> > "map":"dev",
> > "size":33820770304,
> > "uuid":"35018886-397e-4fe7-a348-0a4d16eec44d",
> > "blockdev":"pmem0"
> > }
> > ]
>
> This is because the Ubuntu kernel has the bug that causes _LSR to fail
> so Linux falls back to a namespace defined by the region boundary. On
> that namespace there is an "fsdax" info block located at the region
> base +4K. That info block is tagged with the uuid of
> "35018886-397e-4fe7-a348-0a4d16eec44d".
Thanks for the explanation!
> > I'm trying to find out the correct solution. I apprecite your insights!
>
> It's a mess. First we need to figure out whether the label is actually
> specifying a size of zero, or there is some other bug.
I agree.
> However, the next problem is going to be adding "fsdax" mode support
> on top of the read-only defined namespaces. The ndctl reconfiguration
> flow:
>
>ndctl create-namespace -e namespace0.0 -m fsdax -f
>
> ...will likely fail because deleting the previous namespace in the
> labels is part of that flow. It's always that labels are writable.
>
> Honestly, the quickest path to something functional for Linux is to
> simply delete the _LSR support and use raw mode defined namespaces.
> Why have labels if they are read-only and the region is sufficient for
> defining boundaries?
Just now Hyper-V team confirmed _LSW is not supported.
But with Ubuntu 19.04 kernel (4.18.0-11-generic), I'm able to run the commands
without any issue:
root@decui-gen2-u1904:~# ndctl list
[
{
"dev":"namespace1.0",
"mode":"raw",
"size":137438953472,
"blockdev":"pmem1"
},
{
"dev":"namespace0.0",
"mode":"fsdax",
"map":"dev",
"size":33820770304,
"uuid":"35018886-397e-4fe7-a348-0a4d16eec44d",
"blockdev":"pmem0"
}
]
root@decui-gen2-u1904:~# ndctl destroy-namespace "namespace0.0"
Error: namespace0.0 is active, specify --force for re-configuration
error destroying namespaces: Device or resource busy
destroyed 0 namespaces
root@decui-gen2-u1904:~# ndctl destroy-namespace "namespace0.0" --force
destroyed 1 namespace
root@decui-gen2-u1904:~# ndctl list
[
{
"dev":"namespace1.0",
"mode":"raw",
"size":137438953472,
"blockdev":"pmem1"
}
]
root@decui-gen2-u1904:~# ndctl create-namespace -e namespace0.0 -m fsdax -f
{
[PATCH V2 00/10] X86/KVM/Hyper-V: Add HV ept tlb range list flush support in KVM
From: Lan Tianyu This patchset is to introduce hv ept tlb range list flush function support in the KVM MMU component. Flushing ept tlbs of several address range can be done via single hypercall and new list flush function is used in the kvm_mmu_commit_zap_page() and FNAME(sync_page). This patchset also adds more hv ept tlb range flush support in more KVM MMU function. Change since v1: 1) Make flush list as a hlist instead of list in order to keep struct kvm_mmu_page size. 2) Add last_level flag in the struct kvm_mmu_page instead of spte pointer 3) Move tlb flush from kvm_mmu_notifier_clear_flush_young() to kvm_age_hva() 4) Use range flush in the kvm_vm_ioctl_get/clear_dirty_log() Lan Tianyu (10): X86/Hyper-V: Add parameter offset for hyperv_fill_flush_guest_mapping_list() KVM/VMX: Fill range list in kvm_fill_hv_flush_list_func() KVM/MMU: Add last_level in the struct mmu_spte_page KVM/MMU: Introduce tlb flush with range list KVM/MMU: Flush tlb with range list in sync_page() KVM/MMU: Flush tlb directly in the kvm_mmu_slot_gfn_write_protect() KVM: Add kvm_get_memslot() to get memslot via slot id KVM: Use tlb range flush in the kvm_vm_ioctl_get/clear_dirty_log() KVM: Add flush parameter for kvm_age_hva() KVM/MMU: Use tlb range flush in the kvm_age_hva() arch/arm/include/asm/kvm_host.h | 3 ++- arch/arm64/include/asm/kvm_host.h | 3 ++- arch/mips/include/asm/kvm_host.h| 3 ++- arch/mips/kvm/mmu.c | 11 ++-- arch/powerpc/include/asm/kvm_host.h | 3 ++- arch/powerpc/kvm/book3s.c | 10 ++-- arch/powerpc/kvm/e500_mmu_host.c| 3 ++- arch/x86/hyperv/nested.c| 4 +-- arch/x86/include/asm/kvm_host.h | 11 +++- arch/x86/include/asm/mshyperv.h | 2 +- arch/x86/kvm/mmu.c | 51 + arch/x86/kvm/mmu.h | 7 + arch/x86/kvm/paging_tmpl.h | 15 --- arch/x86/kvm/vmx/vmx.c | 18 +++-- arch/x86/kvm/x86.c | 16 +--- include/linux/kvm_host.h| 1 + virt/kvm/arm/mmu.c | 13 -- virt/kvm/kvm_main.c | 51 +++-- 18 files changed, 160 insertions(+), 65 deletions(-) -- 2.14.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V2 1/10] X86/Hyper-V: Add parameter offset for hyperv_fill_flush_guest_mapping_list()
From: Lan Tianyu
Add parameter offset to specify start position to add flush ranges in
guest address list of struct hv_guest_mapping_flush_list.
Signed-off-by: Lan Tianyu
---
arch/x86/hyperv/nested.c| 4 ++--
arch/x86/include/asm/mshyperv.h | 2 +-
arch/x86/kvm/vmx/vmx.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/hyperv/nested.c b/arch/x86/hyperv/nested.c
index dd0a843f766d..96f8bac7476d 100644
--- a/arch/x86/hyperv/nested.c
+++ b/arch/x86/hyperv/nested.c
@@ -58,11 +58,11 @@ EXPORT_SYMBOL_GPL(hyperv_flush_guest_mapping);
int hyperv_fill_flush_guest_mapping_list(
struct hv_guest_mapping_flush_list *flush,
- u64 start_gfn, u64 pages)
+ int offset, u64 start_gfn, u64 pages)
{
u64 cur = start_gfn;
u64 additional_pages;
- int gpa_n = 0;
+ int gpa_n = offset;
do {
/*
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index cc60e617931c..d6be685ab6b0 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -357,7 +357,7 @@ int hyperv_flush_guest_mapping_range(u64 as,
hyperv_fill_flush_list_func fill_func, void *data);
int hyperv_fill_flush_guest_mapping_list(
struct hv_guest_mapping_flush_list *flush,
- u64 start_gfn, u64 end_gfn);
+ int offset, u64 start_gfn, u64 end_gfn);
#ifdef CONFIG_X86_64
void hv_apic_init(void);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f6915f10e584..9d954b4adce3 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -428,7 +428,7 @@ int kvm_fill_hv_flush_list_func(struct
hv_guest_mapping_flush_list *flush,
{
struct kvm_tlb_range *range = data;
- return hyperv_fill_flush_guest_mapping_list(flush, range->start_gfn,
+ return hyperv_fill_flush_guest_mapping_list(flush, 0, range->start_gfn,
range->pages);
}
--
2.14.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] nfit: add Hyper-V NVDIMM DSM command set to white list
On Fri, Feb 1, 2019 at 5:06 PM Dexuan Cui wrote:
>
> > From: Linux-nvdimm On Behalf Of
> > Dexuan Cui
> > Sent: Friday, February 1, 2019 4:34 PM
> > > > > ...
> > > > > Those reads find a namespace index block
> > > > > and a label. Unfortunately the label has the LOCAL flag set and Linux
> > > > > explicitly ignores pmem namespace labels with that bit set. The reason
> > > > > for that is due to the fact that the original definition of the LOCAL
> > > > > bit from v1.1 of the namespace label implementation [1] explicitly
> > > > > limited the LOCAL flag to "block aperture" regions. If you clear that
> > > > > LOCAL flag I expect it will work. To my knowledge Windows pretends
> > > > > that the v1.1 definition never existed.
>
> On the libnvdimm-pending branch, I get this:
>
> root@decui-gen2-u1904:~/nvdimm# ndctl list
> root@decui-gen2-u1904:~/nvdimm# ndctl list --idle
> [
> {
> "dev":"namespace1.0",
> "mode":"raw",
> "size":0,
> "uuid":"----",
> "state":"disabled"
> },
> {
> "dev":"namespace0.0",
> "mode":"raw",
> "size":0,
> "uuid":"----",
> "state":"disabled"
> }
> ]
>
> With the patch that clears the LOCAL label (BTW, the initial value of flags
> is 0x3,
> meaning a read-only local label) :
> @@ -2496,6 +2500,7 @@ static int init_active_labels(struct nd_region
> *nd_region)
> if (!label_ent)
> break;
> label = nd_label_active(ndd, j);
> + label->flags &= ~NSLABEL_FLAG_LOCAL;
> label_ent->label = label;
>
> I get this:
>
> root@decui-gen2-u1904:~/nvdimm# ndctl list
> root@decui-gen2-u1904:~/nvdimm# ndctl list --idle
> [
> {
> "dev":"namespace1.0",
> "mode":"raw",
> "size":0,
> "uuid":"c258aaab-f72b-e546-bfa5-be5e07761dbc",
> "state":"disabled",
> "name":"Microsoft Hyper-V NVDIMM 1 Label"
> },
> {
> "dev":"namespace0.0",
> "mode":"raw",
> "size":0,
> "uuid":"9f0497a7-4453-7c40-ad35-21a791e00345",
> "state":"disabled",
> "name":"Microsoft Hyper-V NVDIMM 0 Label"
> }
> ]
>
> The "size" and "mode" still don't look right, but the improvement is that
> now I can see a good descriptive "name", which I suppose is retrieved
> from Hyper-V.
Mode is right, there is no way for Hyper-V to create Linux fsdax mode
namespaces it requires some setup using variables only Linux knows.
Can you send the output of:
ndctl read-labels -j all
>
> With Ubuntu 19.04 (4.18.0-11-generic), I get this:
> (Note: the "mode" and "size" are correct. The "uuid" is different from
> the above "9f0497a7-4453-7c40-ad35-21a791e00345" -- this is weird.)
>
> root@decui-gen2-u1904:~# ndctl list
> [
> {
> "dev":"namespace1.0",
> "mode":"raw",
> "size":137438953472,
> "blockdev":"pmem1"
> },
> {
> "dev":"namespace0.0",
> "mode":"fsdax",
> "map":"dev",
> "size":33820770304,
> "uuid":"35018886-397e-4fe7-a348-0a4d16eec44d",
> "blockdev":"pmem0"
> }
> ]
This is because the Ubuntu kernel has the bug that causes _LSR to fail
so Linux falls back to a namespace defined by the region boundary. On
that namespace there is an "fsdax" info block located at the region
base +4K. That info block is tagged with the uuid of
"35018886-397e-4fe7-a348-0a4d16eec44d".
> I'm trying to find out the correct solution. I apprecite your insights!
It's a mess. First we need to figure out whether the label is actually
specifying a size of zero, or there is some other bug.
However, the next problem is going to be adding "fsdax" mode support
on top of the read-only defined namespaces. The ndctl reconfiguration
flow:
ndctl create-namespace -e namespace0.0 -m fsdax -f
...will likely fail because deleting the previous namespace in the
labels is part of that flow. It's always that labels are writable.
Honestly, the quickest path to something functional for Linux is to
simply delete the _LSR support and use raw mode defined namespaces.
Why have labels if they are read-only and the region is sufficient for
defining boundaries?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2] nfit: add Hyper-V NVDIMM DSM command set to white list
> From: Linux-nvdimm On Behalf Of
> Dexuan Cui
> Sent: Friday, February 1, 2019 4:34 PM
> > > > ...
> > > > Those reads find a namespace index block
> > > > and a label. Unfortunately the label has the LOCAL flag set and Linux
> > > > explicitly ignores pmem namespace labels with that bit set. The reason
> > > > for that is due to the fact that the original definition of the LOCAL
> > > > bit from v1.1 of the namespace label implementation [1] explicitly
> > > > limited the LOCAL flag to "block aperture" regions. If you clear that
> > > > LOCAL flag I expect it will work. To my knowledge Windows pretends
> > > > that the v1.1 definition never existed.
On the libnvdimm-pending branch, I get this:
root@decui-gen2-u1904:~/nvdimm# ndctl list
root@decui-gen2-u1904:~/nvdimm# ndctl list --idle
[
{
"dev":"namespace1.0",
"mode":"raw",
"size":0,
"uuid":"----",
"state":"disabled"
},
{
"dev":"namespace0.0",
"mode":"raw",
"size":0,
"uuid":"----",
"state":"disabled"
}
]
With the patch that clears the LOCAL label (BTW, the initial value of flags is
0x3,
meaning a read-only local label) :
@@ -2496,6 +2500,7 @@ static int init_active_labels(struct nd_region *nd_region)
if (!label_ent)
break;
label = nd_label_active(ndd, j);
+ label->flags &= ~NSLABEL_FLAG_LOCAL;
label_ent->label = label;
I get this:
root@decui-gen2-u1904:~/nvdimm# ndctl list
root@decui-gen2-u1904:~/nvdimm# ndctl list --idle
[
{
"dev":"namespace1.0",
"mode":"raw",
"size":0,
"uuid":"c258aaab-f72b-e546-bfa5-be5e07761dbc",
"state":"disabled",
"name":"Microsoft Hyper-V NVDIMM 1 Label"
},
{
"dev":"namespace0.0",
"mode":"raw",
"size":0,
"uuid":"9f0497a7-4453-7c40-ad35-21a791e00345",
"state":"disabled",
"name":"Microsoft Hyper-V NVDIMM 0 Label"
}
]
The "size" and "mode" still don't look right, but the improvement is that
now I can see a good descriptive "name", which I suppose is retrieved
from Hyper-V.
With Ubuntu 19.04 (4.18.0-11-generic), I get this:
(Note: the "mode" and "size" are correct. The "uuid" is different from
the above "9f0497a7-4453-7c40-ad35-21a791e00345" -- this is weird.)
root@decui-gen2-u1904:~# ndctl list
[
{
"dev":"namespace1.0",
"mode":"raw",
"size":137438953472,
"blockdev":"pmem1"
},
{
"dev":"namespace0.0",
"mode":"fsdax",
"map":"dev",
"size":33820770304,
"uuid":"35018886-397e-4fe7-a348-0a4d16eec44d",
"blockdev":"pmem0"
}
]
I'm trying to find out the correct solution. I apprecite your insights!
Thanks,
-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] nfit: add Hyper-V NVDIMM DSM command set to white list
On Fri, Feb 1, 2019 at 4:34 PM Dexuan Cui wrote: > > > From: Dan Williams > > Sent: Friday, February 1, 2019 3:47 PM > > To: Dexuan Cui > > > > I believe it's the same reason. Without 11189c1089da the _LSR method > > will fail, and otherwise it works and finds the label that it doesn't > > like. > Exactly. > > > I'm not seeing "invalid" data in your failure log. Could you double > > check that it's just not the success of _LSR that causes the issue? > > acpi_label_read() never fails for me. > > By "invalid", I only mean the messages in the dmesg.bad.txt I previously > attached (I'm just reading the specs to learn the details about NVDIMM > namespace's labels, so my description might be inaccurate) : > > [4.832367] nvdimm nmem1: nsindex0 labelsize 1 invalid > [4.832369] nvdimm nmem1: nsindex1 labelsize 1 invalid Oh, those are benign. They are a side effect of Linux probing for v1.2 namespace labels vs v1.1. It will always find that one of those is "invalid". > ... > [5.259017] nd_pmem namespace0.0: 0x, too small must be at > least 0x1000 > > > > > The regression you are seeing is the fact that the patch enables the > > > > kernel > > to > > > > enable nvdimm-namespace-label reads. > > > Yes. > > > > > > > Those reads find a namespace index block > > > > and a label. Unfortunately the label has the LOCAL flag set and Linux > > > > explicitly ignores pmem namespace labels with that bit set. The reason > > > Can you please point out the function that ignores the flag? > > > > > > I checked where NSLABEL_FLAG_LOCAL is used, but it looks I can't find a > > > related function. > > > > scan_labels() is where the namespace label is validated relative to > > the region type: > > > > if (is_nd_blk(_region->dev) > > == !!(flags & NSLABEL_FLAG_LOCAL)) > > /* pass, region matches label type */; > > else > > continue; > > > > It also has meaning for the namespace capacity allocation > > implementation that needed that flag to distinguish aliased capacity > > between Block Aperture Mode and PMEM Mode access. > Thanks for the pointer! I'm looking at this function. > > > > > for that is due to the fact that the original definition of the LOCAL > > > > bit from v1.1 of the namespace label implementation [1] explicitly > > > > limited the LOCAL flag to "block aperture" regions. If you clear that > > > > LOCAL flag I expect it will work. To my knowledge Windows pretends > > > > that the v1.1 definition never existed. > > > I'm trying to find out where the flag is used and how to clear it. > > > > Assuming Hyper-V implements _LSW, you can recreate / reinitialize the > > label area: > > I think Hyper-V only implements _LSR: > [4.720623] nfit ACPI0012:00: device:00: has _LSR > [4.723683] nfit ACPI0012:00: device:01: has _LSR That's unfortunate... > > > > > The UEFI 2.7 specification for v1.2 labels states that setting the > > > > LOCAL flag is optional when "nlabel", number of labels in the set, is > > > > 1. Linux makes that mandatory as LOCAL is redundant when nlabel is 1. > > > > > > > > That said, the Robustness Principle makes a case that Linux should > > > > tolerate the bit being set. However, it's just a non-trivial amount of > > > > work to unwind the ingrained block-aperture assumptions of that bit. > > > Can you please explain this a bit more? Sorry, I'm new to this area... > > > > The short story is that Linux enforces that LOCAL == Block Mode > > Namespaces. See section 2.2 Namespace Label Layout in the original > > spec [1]. The EFI 2.7 definition tried to allow for LOCAL to be set > > when an interleave-set was comprised of a single NVDIMM, but then also > > states its optional when Nlabel is 1. It has zero functional use for > > interleave-set based namespaces even when the interleave-set-width is > > 1. So Linux takes the option to never set it, and goes further to > > reject it if it's set and the region-type does not match, because that > > follows the v1.1 meaning of the flag. > > > > [1]: > Thanks for the link! I'll read it. > BTW, it looks Hyper-V only supports PMEM namespace, at least so far. I don't think it should bother. It only makes sense for bare metal and even then I know of no NVDIMMs that are shipping it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2] nfit: add Hyper-V NVDIMM DSM command set to white list
> From: Dan Williams > Sent: Friday, February 1, 2019 3:47 PM > To: Dexuan Cui > > I believe it's the same reason. Without 11189c1089da the _LSR method > will fail, and otherwise it works and finds the label that it doesn't > like. Exactly. > I'm not seeing "invalid" data in your failure log. Could you double > check that it's just not the success of _LSR that causes the issue? acpi_label_read() never fails for me. By "invalid", I only mean the messages in the dmesg.bad.txt I previously attached (I'm just reading the specs to learn the details about NVDIMM namespace's labels, so my description might be inaccurate) : [4.832367] nvdimm nmem1: nsindex0 labelsize 1 invalid [4.832369] nvdimm nmem1: nsindex1 labelsize 1 invalid ... [5.259017] nd_pmem namespace0.0: 0x, too small must be at least 0x1000 > > > The regression you are seeing is the fact that the patch enables the > > > kernel > to > > > enable nvdimm-namespace-label reads. > > Yes. > > > > > Those reads find a namespace index block > > > and a label. Unfortunately the label has the LOCAL flag set and Linux > > > explicitly ignores pmem namespace labels with that bit set. The reason > > Can you please point out the function that ignores the flag? > > > > I checked where NSLABEL_FLAG_LOCAL is used, but it looks I can't find a > > related function. > > scan_labels() is where the namespace label is validated relative to > the region type: > > if (is_nd_blk(_region->dev) > == !!(flags & NSLABEL_FLAG_LOCAL)) > /* pass, region matches label type */; > else > continue; > > It also has meaning for the namespace capacity allocation > implementation that needed that flag to distinguish aliased capacity > between Block Aperture Mode and PMEM Mode access. Thanks for the pointer! I'm looking at this function. > > > for that is due to the fact that the original definition of the LOCAL > > > bit from v1.1 of the namespace label implementation [1] explicitly > > > limited the LOCAL flag to "block aperture" regions. If you clear that > > > LOCAL flag I expect it will work. To my knowledge Windows pretends > > > that the v1.1 definition never existed. > > I'm trying to find out where the flag is used and how to clear it. > > Assuming Hyper-V implements _LSW, you can recreate / reinitialize the > label area: I think Hyper-V only implements _LSR: [4.720623] nfit ACPI0012:00: device:00: has _LSR [4.723683] nfit ACPI0012:00: device:01: has _LSR > > > The UEFI 2.7 specification for v1.2 labels states that setting the > > > LOCAL flag is optional when "nlabel", number of labels in the set, is > > > 1. Linux makes that mandatory as LOCAL is redundant when nlabel is 1. > > > > > > That said, the Robustness Principle makes a case that Linux should > > > tolerate the bit being set. However, it's just a non-trivial amount of > > > work to unwind the ingrained block-aperture assumptions of that bit. > > Can you please explain this a bit more? Sorry, I'm new to this area... > > The short story is that Linux enforces that LOCAL == Block Mode > Namespaces. See section 2.2 Namespace Label Layout in the original > spec [1]. The EFI 2.7 definition tried to allow for LOCAL to be set > when an interleave-set was comprised of a single NVDIMM, but then also > states its optional when Nlabel is 1. It has zero functional use for > interleave-set based namespaces even when the interleave-set-width is > 1. So Linux takes the option to never set it, and goes further to > reject it if it's set and the region-type does not match, because that > follows the v1.1 meaning of the flag. > > [1]: Thanks for the link! I'll read it. BTW, it looks Hyper-V only supports PMEM namespace, at least so far. Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] nfit: add Hyper-V NVDIMM DSM command set to white list
On Fri, Feb 1, 2019 at 3:17 PM Dexuan Cui wrote:
>
> > From: Dan Williams
> > Sent: Friday, February 1, 2019 9:29 AM
> > > Hi Dan,
> > > Unluckily it looks this commit causes a regression ...
> > > With the patch, "ndctl list" shows nothing, and /dev/pmem0 can't appear.
> > > If I revert the patch, it will be back to normal.
> > >
> > > I attached the config/logs. In the bad case, "dmesg" shows a line
> > > [5.259017] nd_pmem namespace0.0: 0x, too small
> > must be at least 0x1000
> > > Any idea why this happens? I'm digging into the details and I appreciate
> > > your
> > insights.
> >
> > Looks like it is working as expected.
>
> I was working on linux-next tree's next-20190107 and this patch did "work
> fine"
> there. The "regression" happens on djbw/nvdimm.git tree's libnvdimm-pending
> branch because we have this recent commit (Jan 19):
>
> 11189c1089da ("acpi/nfit: Fix command-supported detection"), which makes such
> a change in acpi_nfit_ctl():
>
> - if (!test_bit(cmd, _mask) || !test_bit(func, _mask))
> + if (cmd == ND_CMD_CALL && !test_bit(func, _mask))
> + return -ENOTTY;
> + else if (!test_bit(cmd, _mask))
> return -ENOTTY;
>
> So previously ND_CMD_GET_CONFIG_DATA fails with -ENOTTY and we're good.
>
> Now the command succeeds, but it looks the returned data is inavlid, and I see
> the "regression".
I believe it's the same reason. Without 11189c1089da the _LSR method
will fail, and otherwise it works and finds the label that it doesn't
like.
I'm not seeing "invalid" data in your failure log. Could you double
check that it's just not the success of _LSR that causes the issue?
> > The regression you are seeing is the fact that the patch enables the kernel
> > to
> > enable nvdimm-namespace-label reads.
> Yes.
>
> > Those reads find a namespace index block
> > and a label. Unfortunately the label has the LOCAL flag set and Linux
> > explicitly ignores pmem namespace labels with that bit set. The reason
> Can you please point out the function that ignores the flag?
>
> I checked where NSLABEL_FLAG_LOCAL is used, but it looks I can't find a
> related function.
scan_labels() is where the namespace label is validated relative to
the region type:
if (is_nd_blk(_region->dev)
== !!(flags & NSLABEL_FLAG_LOCAL))
/* pass, region matches label type */;
else
continue;
It also has meaning for the namespace capacity allocation
implementation that needed that flag to distinguish aliased capacity
between Block Aperture Mode and PMEM Mode access.
> > for that is due to the fact that the original definition of the LOCAL
> > bit from v1.1 of the namespace label implementation [1] explicitly
> > limited the LOCAL flag to "block aperture" regions. If you clear that
> > LOCAL flag I expect it will work. To my knowledge Windows pretends
> > that the v1.1 definition never existed.
> I'm trying to find out where the flag is used and how to clear it.
Assuming Hyper-V implements _LSW, you can recreate / reinitialize the
label area:
ndctl disable-region all
ndctl init-labels -f all
ndctl enable-region all
ndctl create-namespace
> > The UEFI 2.7 specification for v1.2 labels states that setting the
> > LOCAL flag is optional when "nlabel", number of labels in the set, is
> > 1. Linux makes that mandatory as LOCAL is redundant when nlabel is 1.
> >
> > That said, the Robustness Principle makes a case that Linux should
> > tolerate the bit being set. However, it's just a non-trivial amount of
> > work to unwind the ingrained block-aperture assumptions of that bit.
> Can you please explain this a bit more? Sorry, I'm new to this area...
The short story is that Linux enforces that LOCAL == Block Mode
Namespaces. See section 2.2 Namespace Label Layout in the original
spec [1]. The EFI 2.7 definition tried to allow for LOCAL to be set
when an interleave-set was comprised of a single NVDIMM, but then also
states its optional when Nlabel is 1. It has zero functional use for
interleave-set based namespaces even when the interleave-set-width is
1. So Linux takes the option to never set it, and goes further to
reject it if it's set and the region-type does not match, because that
follows the v1.1 meaning of the flag.
[1]: http://pmem.io/documents/NVDIMM_Namespace_Spec.pdf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2] nfit: add Hyper-V NVDIMM DSM command set to white list
> From: Dan Williams
> Sent: Friday, February 1, 2019 9:29 AM
> > Hi Dan,
> > Unluckily it looks this commit causes a regression ...
> > With the patch, "ndctl list" shows nothing, and /dev/pmem0 can't appear.
> > If I revert the patch, it will be back to normal.
> >
> > I attached the config/logs. In the bad case, "dmesg" shows a line
> > [5.259017] nd_pmem namespace0.0: 0x, too small
> must be at least 0x1000
> > Any idea why this happens? I'm digging into the details and I appreciate
> > your
> insights.
>
> Looks like it is working as expected.
I was working on linux-next tree's next-20190107 and this patch did "work fine"
there. The "regression" happens on djbw/nvdimm.git tree's libnvdimm-pending
branch because we have this recent commit (Jan 19):
11189c1089da ("acpi/nfit: Fix command-supported detection"), which makes such
a change in acpi_nfit_ctl():
- if (!test_bit(cmd, _mask) || !test_bit(func, _mask))
+ if (cmd == ND_CMD_CALL && !test_bit(func, _mask))
+ return -ENOTTY;
+ else if (!test_bit(cmd, _mask))
return -ENOTTY;
So previously ND_CMD_GET_CONFIG_DATA fails with -ENOTTY and we're good.
Now the command succeeds, but it looks the returned data is inavlid, and I see
the "regression".
> The regression you are seeing is the fact that the patch enables the kernel to
> enable nvdimm-namespace-label reads.
Yes.
> Those reads find a namespace index block
> and a label. Unfortunately the label has the LOCAL flag set and Linux
> explicitly ignores pmem namespace labels with that bit set. The reason
Can you please point out the function that ignores the flag?
I checked where NSLABEL_FLAG_LOCAL is used, but it looks I can't find a
related function.
> for that is due to the fact that the original definition of the LOCAL
> bit from v1.1 of the namespace label implementation [1] explicitly
> limited the LOCAL flag to "block aperture" regions. If you clear that
> LOCAL flag I expect it will work. To my knowledge Windows pretends
> that the v1.1 definition never existed.
I'm trying to find out where the flag is used and how to clear it.
> The UEFI 2.7 specification for v1.2 labels states that setting the
> LOCAL flag is optional when "nlabel", number of labels in the set, is
> 1. Linux makes that mandatory as LOCAL is redundant when nlabel is 1.
>
> That said, the Robustness Principle makes a case that Linux should
> tolerate the bit being set. However, it's just a non-trivial amount of
> work to unwind the ingrained block-aperture assumptions of that bit.
Can you please explain this a bit more? Sorry, I'm new to this area...
Thanks,
-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC net-next 06/13] staging: fsl-dpaa2: ethsw: Handle SWITCHDEV_PORT_ATTR_GET/SET
Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.
Prepare ethsw to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET/SET and simply translate that into the existing
swdev_port_attr_{set,get} calls.
Signed-off-by: Florian Fainelli
---
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 24
1 file changed, 24 insertions(+)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index daabaceeea52..b18e112d84e6 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -1103,6 +1103,27 @@ ethsw_switchdev_port_obj_event(unsigned long event,
struct net_device *netdev,
return notifier_from_errno(err);
}
+static int
+ethsw_switchdev_port_attr_event(unsigned long event,
+ struct net_device *netdev,
+ struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+ int err = -EOPNOTSUPP;
+
+ switch (event) {
+ case SWITCHDEV_PORT_ATTR_SET:
+ err = swdev_port_attr_set(netdev, port_attr_info->attr,
+ port_attr_info->trans);
+ break;
+ case SWITCHDEV_PORT_ATTR_GET:
+ err = swdev_port_attr_get(netdev, port_attr_info->attr);
+ break;
+ }
+
+ port_attr_info->handled = true;
+ return notifier_from_errno(err);
+}
+
static int port_switchdev_blocking_event(struct notifier_block *unused,
unsigned long event, void *ptr)
{
@@ -1115,6 +1136,9 @@ static int port_switchdev_blocking_event(struct
notifier_block *unused,
case SWITCHDEV_PORT_OBJ_ADD: /* fall through */
case SWITCHDEV_PORT_OBJ_DEL:
return ethsw_switchdev_port_obj_event(event, dev, ptr);
+ case SWITCHDEV_PORT_ATTR_SET:
+ case SWITCHDEV_PORT_ATTR_GET:
+ return ethsw_switchdev_port_attr_event(event, dev, ptr);
}
return NOTIFY_DONE;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC net-next 09/13] mlxsw: switchx2: Handle SWITCHDEV_PORT_ATTR_GET event
Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.
Prepare bnxt to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET and simply translate that into the existing
switchdev_ops::switchdev_port_attr_get operation.
We register a single blocking switchdev notifier for the entire driver
instance and check that thet network device maps to the one that
switchx2 manages.
Signed-off-by: Florian Fainelli
---
.../net/ethernet/mellanox/mlxsw/switchx2.c| 43 ++-
1 file changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index 2d4f213e154d..82fb8f1bb6e9 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -923,6 +923,36 @@ static const struct switchdev_ops
mlxsw_sx_port_switchdev_ops = {
.switchdev_port_attr_get= mlxsw_sx_port_attr_get,
};
+static int mlxsw_sx_port_attr_event(unsigned long event,
+ struct net_device *dev,
+ struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+ int rc;
+
+ if (event != SWITCHDEV_PORT_ATTR_GET)
+ return NOTIFY_DONE;
+
+ rc = mlxsw_sx_port_attr_get(dev, port_attr_info->attr);
+ port_attr_info->handled = true;
+ return notifier_from_errno(rc);
+}
+
+static int mlxsw_sx_swdev_blocking_event(struct notifier_block *nb,
+unsigned long event, void *ptr)
+{
+ struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+
+ if (dev->netdev_ops != _sx_port_netdev_ops)
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case SWITCHDEV_PORT_ATTR_GET:
+ return mlxsw_sx_port_attr_event(event, dev, ptr);
+ }
+
+ return NOTIFY_DONE;
+}
+
static int mlxsw_sx_hw_id_get(struct mlxsw_sx *mlxsw_sx)
{
char spad_pl[MLXSW_REG_SPAD_LEN] = {0};
@@ -1638,6 +1668,10 @@ static void mlxsw_sx_fini(struct mlxsw_core *mlxsw_core)
mlxsw_sx_ports_remove(mlxsw_sx);
}
+static struct notifier_block mlxsw_sx_swdev_nb = {
+ .notifier_call = mlxsw_sx_swdev_blocking_event,
+};
+
static const struct mlxsw_config_profile mlxsw_sx_config_profile = {
.used_max_vepa_channels = 1,
.max_vepa_channels = 0,
@@ -1698,10 +1732,14 @@ static int __init mlxsw_sx_module_init(void)
{
int err;
- err = mlxsw_core_driver_register(_sx_driver);
+ err = register_switchdev_blocking_notifier(_sx_swdev_nb);
if (err)
return err;
+ err = mlxsw_core_driver_register(_sx_driver);
+ if (err)
+ goto err_unregister_notifier;
+
err = mlxsw_pci_driver_register(_sx_pci_driver);
if (err)
goto err_pci_driver_register;
@@ -1710,6 +1748,8 @@ static int __init mlxsw_sx_module_init(void)
err_pci_driver_register:
mlxsw_core_driver_unregister(_sx_driver);
+err_unregister_notifier:
+ unregister_switchdev_blocking_notifier(_sx_swdev_nb);
return err;
}
@@ -1717,6 +1757,7 @@ static void __exit mlxsw_sx_module_exit(void)
{
mlxsw_pci_driver_unregister(_sx_pci_driver);
mlxsw_core_driver_unregister(_sx_driver);
+ unregister_switchdev_blocking_notifier(_sx_swdev_nb);
}
module_init(mlxsw_sx_module_init);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC net-next 12/13] netdevsim: Handle SWITCHDEV_PORT_ATTR_GET event
Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.
Prepare netdevsim to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET and simply translate that into the existing
switchdev_ops::switchdev_port_attr_get operation.
We register a single blocking switchdev notifier for the entire driver
instance.
Signed-off-by: Florian Fainelli
---
drivers/net/netdevsim/netdev.c | 43 +-
1 file changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 8d8e2b3f263e..817d94cec90f 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -168,6 +168,20 @@ static const struct switchdev_ops nsim_switchdev_ops = {
.switchdev_port_attr_get= nsim_port_attr_get,
};
+static int nsim_switchdev_port_attr_event(unsigned long event,
+ struct net_device *dev,
+ struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+ int rc;
+
+ if (event != SWITCHDEV_PORT_ATTR_GET)
+ return NOTIFY_DONE;
+
+ rc = nsim_port_attr_get(dev, port_attr_info->attr);
+ port_attr_info->handled = true;
+ return notifier_from_errno(rc);
+}
+
static int nsim_init(struct net_device *dev)
{
char sdev_ddir_name[10], sdev_link_name[32];
@@ -495,6 +509,26 @@ static const struct net_device_ops nsim_netdev_ops = {
.ndo_bpf= nsim_bpf,
};
+static int nsim_swdev_blocking_event(struct notifier_block *nb,
+unsigned long event, void *ptr)
+{
+ struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+
+ if (dev->netdev_ops != _netdev_ops)
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case SWITCHDEV_PORT_ATTR_GET:
+ return nsim_switchdev_port_attr_event(event, dev, ptr);
+ }
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block nsim_swdev_blocking_nb = {
+ .notifier_call = nsim_swdev_blocking_event,
+};
+
static void nsim_setup(struct net_device *dev)
{
ether_setup(dev);
@@ -583,10 +617,14 @@ static int __init nsim_module_init(void)
goto err_debugfs_destroy;
}
- err = bus_register(_bus);
+ err = register_switchdev_blocking_notifier(_swdev_blocking_nb);
if (err)
goto err_sdir_destroy;
+ err = bus_register(_bus);
+ if (err)
+ goto err_unreg_notifier;
+
err = nsim_devlink_init();
if (err)
goto err_unreg_bus;
@@ -601,6 +639,8 @@ static int __init nsim_module_init(void)
nsim_devlink_exit();
err_unreg_bus:
bus_unregister(_bus);
+err_unreg_notifier:
+ unregister_switchdev_blocking_notifier(_swdev_blocking_nb);
err_sdir_destroy:
debugfs_remove_recursive(nsim_sdev_ddir);
err_debugfs_destroy:
@@ -613,6 +653,7 @@ static void __exit nsim_module_exit(void)
rtnl_link_unregister(_link_ops);
nsim_devlink_exit();
bus_unregister(_bus);
+ unregister_switchdev_blocking_notifier(_swdev_blocking_nb);
debugfs_remove_recursive(nsim_sdev_ddir);
debugfs_remove_recursive(nsim_ddir);
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC net-next 04/13] mlxsw: spectrum_switchdev: Handle SWITCHDEV_PORT_ATTR_GET/SET
Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.
Prepare mlxsw to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET/SET and simply translate that into the existing
mlxsw_sp_port_attr_{set,get} calls.
Signed-off-by: Florian Fainelli
---
.../mellanox/mlxsw/spectrum_switchdev.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 0f4e68d31cc3..7c0df736587c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -3442,6 +3442,26 @@ mlxsw_sp_switchdev_handle_vxlan_obj_del(struct
net_device *vxlan_dev,
}
}
+static int
+mlxsw_sp_switchdev_port_attr_event(unsigned long event, struct net_device *dev,
+ struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+ int err = -EOPNOTSUPP;
+
+ switch (event) {
+ case SWITCHDEV_PORT_ATTR_SET:
+ err = mlxsw_sp_port_attr_set(dev, port_attr_info->attr,
+port_attr_info->trans);
+ break;
+ case SWITCHDEV_PORT_ATTR_GET:
+ err = mlxsw_sp_port_attr_get(dev, port_attr_info->attr);
+ break;
+ }
+
+ port_attr_info->handled = true;
+ return notifier_from_errno(err);
+}
+
static int mlxsw_sp_switchdev_blocking_event(struct notifier_block *unused,
unsigned long event, void *ptr)
{
@@ -3465,6 +3485,9 @@ static int mlxsw_sp_switchdev_blocking_event(struct
notifier_block *unused,
mlxsw_sp_port_dev_check,
mlxsw_sp_port_obj_del);
return notifier_from_errno(err);
+ case SWITCHDEV_PORT_ATTR_SET:
+ case SWITCHDEV_PORT_ATTR_GET:
+ return mlxsw_sp_switchdev_port_attr_event(event, dev, ptr);
}
return NOTIFY_DONE;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC net-next 08/13] liquidio: Handle SWITCHDEV_PORT_ATTR_GET event
Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.
Prepare bnxt to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET and simply translate that into the existing
switchdev_ops::switchdev_port_attr_get operation.
We register a single blocking switchdev notifier for the PF part of the
driver, and we register another blocking switchdev notifier, following
what was done with the existing netdevice notifier within the VF
representor driver.
Signed-off-by: Florian Fainelli
---
.../net/ethernet/cavium/liquidio/lio_main.c | 48 ++-
.../net/ethernet/cavium/liquidio/lio_vf_rep.c | 45 -
2 files changed, 91 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c
b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index 3d24133e5e49..b9d48e4181fc 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -162,6 +162,8 @@ static int liquidio_set_vf_link_state(struct net_device
*netdev, int vfidx,
static struct handshake handshake[MAX_OCTEON_DEVICES];
static struct completion first_stage;
+static int liquidio_switchdev_blocking_event(struct notifier_block *nb,
+unsigned long event, void *ptr);
static void octeon_droq_bh(unsigned long pdev)
{
@@ -469,12 +471,25 @@ static struct pci_driver liquidio_pci_driver = {
#endif
};
+static struct notifier_block liquidio_blocking_nb = {
+ .notifier_call = liquidio_switchdev_blocking_event,
+};
+
/**
* \brief register PCI driver
*/
static int liquidio_init_pci(void)
{
- return pci_register_driver(_pci_driver);
+ int rc;
+
+ rc = register_switchdev_blocking_notifier(_blocking_nb);
+ if (rc)
+ return rc;
+ rc = pci_register_driver(_pci_driver);
+ if (rc)
+ unregister_switchdev_blocking_notifier(_blocking_nb);
+
+ return rc;
}
/**
@@ -483,6 +498,7 @@ static int liquidio_init_pci(void)
static void liquidio_deinit_pci(void)
{
pci_unregister_driver(_pci_driver);
+ unregister_switchdev_blocking_notifier(_blocking_nb);
}
/**
@@ -3261,6 +3277,36 @@ static const struct net_device_ops lionetdevops = {
.ndo_get_vf_stats = liquidio_get_vf_stats,
};
+static int lio_pf_attr_event(unsigned long event, struct net_device *dev,
+ struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+ int rc;
+
+ if (event != SWITCHDEV_PORT_ATTR_GET)
+ return NOTIFY_DONE;
+
+ rc = lio_pf_switchdev_attr_get(dev, port_attr_info->attr);
+
+ port_attr_info->handled = true;
+ return notifier_from_errno(rc);
+}
+
+static int liquidio_switchdev_blocking_event(struct notifier_block *nb,
+unsigned long event, void *ptr)
+{
+ struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+
+ if (dev->netdev_ops != )
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case SWITCHDEV_PORT_ATTR_GET:
+ return lio_pf_attr_event(event, dev, ptr);
+ }
+
+ return NOTIFY_DONE;
+}
+
/** \brief Entry point for the liquidio module
*/
static int __init liquidio_init(void)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c
b/drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c
index de61060721c4..d396c004c1be 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c
@@ -468,6 +468,21 @@ static const struct switchdev_ops lio_vf_rep_switchdev_ops
= {
.switchdev_port_attr_get= lio_vf_rep_attr_get,
};
+static int lio_vf_rep_swdev_port_attr_event(unsigned long event,
+ struct net_device *dev,
+ struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+ int rc;
+
+ if (event != SWITCHDEV_PORT_ATTR_GET)
+ return NOTIFY_DONE;
+
+ rc = lio_vf_rep_attr_get(dev, port_attr_info->attr);
+ port_attr_info->handled = true;
+
+ return notifier_from_errno(rc);
+}
+
static void
lio_vf_rep_fetch_stats(struct work_struct *work)
{
@@ -538,7 +553,6 @@ lio_vf_rep_create(struct octeon_device *oct)
if (register_netdev(ndev)) {
dev_err(>pci_dev->dev, "VF rep nerdev registration
failed\n");
-
free_netdev(ndev);
goto cleanup;
}
@@ -664,20 +678,49 @@ static struct notifier_block lio_vf_rep_netdev_notifier =
{
.notifier_call = lio_vf_rep_netdev_event,
};
+static int lio_vf_rep_swdev_event(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct net_device *ndev = switchdev_notifier_info_to_dev(ptr);
+
+ if (ndev->netdev_ops != _vf_rep_ndev_ops)
+
[RFC net-next 11/13] nfp: Handle SWITCHDEV_PORT_ATTR_GET event
Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.
Prepare nfp to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET and simply translate that into the existing
switchdev_ops::switchdev_port_attr_get operation.
We register a single blocking switchdev notifier for the entire driver
instance and we differentiate a "net" from a "repr" by comparing the
network device's netdev_ops with the ones that this driver manages.
Signed-off-by: Florian Fainelli
---
drivers/net/ethernet/netronome/nfp/nfp_main.c | 14 +-
drivers/net/ethernet/netronome/nfp/nfp_net.h| 3 +++
.../net/ethernet/netronome/nfp/nfp_net_common.c | 17 +
drivers/net/ethernet/netronome/nfp/nfp_port.c | 15 +++
drivers/net/ethernet/netronome/nfp/nfp_port.h | 5 -
5 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c
b/drivers/net/ethernet/netronome/nfp/nfp_main.c
index 6c10e8d119e4..50c111280cd4 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include "nfpcore/nfp.h"
#include "nfpcore/nfp_cpp.h"
@@ -708,6 +709,10 @@ static void nfp_pci_remove(struct pci_dev *pdev)
pci_disable_device(pdev);
}
+static struct notifier_block nfp_swdev_blocking_nb = {
+ .notifier_call = nfp_switchdev_blocking_event,
+};
+
static struct pci_driver nfp_pci_driver = {
.name = nfp_driver_name,
.id_table = nfp_pci_device_ids,
@@ -725,9 +730,13 @@ static int __init nfp_main_init(void)
nfp_net_debugfs_create();
+ err = register_switchdev_blocking_notifier(_swdev_blocking_nb);
+ if (err)
+ goto err_destroy_debugfs;
+
err = pci_register_driver(_pci_driver);
if (err < 0)
- goto err_destroy_debugfs;
+ goto err_unreg_notifier;
err = pci_register_driver(_netvf_pci_driver);
if (err)
@@ -737,6 +746,8 @@ static int __init nfp_main_init(void)
err_unreg_pf:
pci_unregister_driver(_pci_driver);
+err_unreg_notifier:
+ unregister_switchdev_blocking_notifier(_swdev_blocking_nb);
err_destroy_debugfs:
nfp_net_debugfs_destroy();
return err;
@@ -746,6 +757,7 @@ static void __exit nfp_main_exit(void)
{
pci_unregister_driver(_netvf_pci_driver);
pci_unregister_driver(_pci_driver);
+ unregister_switchdev_blocking_notifier(_swdev_blocking_nb);
nfp_net_debugfs_destroy();
}
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h
b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index be37c2d6151c..57f7d6d634ea 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -915,4 +915,7 @@ static inline void nfp_net_debugfs_dir_clean(struct dentry
**dir)
}
#endif /* CONFIG_NFP_DEBUG */
+int nfp_switchdev_blocking_event(struct notifier_block *nb,
+unsigned long event, void *ptr);
+
#endif /* _NFP_NET_H_ */
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 7d2d4241498f..c2c5e7e3aab0 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3955,3 +3955,20 @@ void nfp_net_clean(struct nfp_net *nn)
unregister_netdev(nn->dp.netdev);
nfp_net_reconfig_wait_posted(nn);
}
+
+int nfp_switchdev_blocking_event(struct notifier_block *nb,
+unsigned long event, void *ptr)
+{
+ struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+
+ if (!nfp_netdev_is_nfp_repr(dev) &&
+ !nfp_netdev_is_nfp_net(dev))
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case SWITCHDEV_PORT_ATTR_GET:
+ return nfp_port_switchdev_attr_event(event, dev, ptr);
+ }
+
+ return NOTIFY_DONE;
+}
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_port.c
b/drivers/net/ethernet/netronome/nfp/nfp_port.c
index 86bc149ca231..578477242ae9 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_port.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_port.c
@@ -59,6 +59,21 @@ const struct switchdev_ops nfp_port_switchdev_ops = {
.switchdev_port_attr_get= nfp_port_attr_get,
};
+int nfp_port_switchdev_attr_event(unsigned long event,
+ struct net_device *netdev,
+ struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+ int rc;
+
+ if (event != SWITCHDEV_PORT_ATTR_GET)
+ return NOTIFY_DONE;
+
+ rc = nfp_port_attr_get(netdev, port_attr_info->attr);
+ port_attr_info->handled = true;
+
+ return notifier_from_errno(rc);
+}
+
int
[RFC net-next 10/13] net/mlx5e: Handle SWITCHDEV_PORT_ATTR_GET event
Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.
Prepare mlx5e/en_rep.c to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET and simply translate that into the existing
switchdev_ops::switchdev_port_attr_get operation.
We register a single blocking switchdev notifier for the entire set of
representors given that mlx5e_rep_register_vport_reps() gets called for
an Ethernet device.
Signed-off-by: Florian Fainelli
---
.../net/ethernet/mellanox/mlx5/core/en_main.c | 4 +-
.../net/ethernet/mellanox/mlx5/core/en_rep.c | 45 ++-
.../net/ethernet/mellanox/mlx5/core/en_rep.h | 2 +-
3 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index dee0c8f3d4e9..fcfe1c9d3575 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -5036,7 +5036,9 @@ static void *mlx5e_add(struct mlx5_core_dev *mdev)
#ifdef CONFIG_MLX5_ESWITCH
if (MLX5_ESWITCH_MANAGER(mdev) &&
mlx5_eswitch_mode(mdev->priv.eswitch) == SRIOV_OFFLOADS) {
- mlx5e_rep_register_vport_reps(mdev);
+ err = mlx5e_rep_register_vport_reps(mdev);
+ if (err)
+ return NULL;
return mdev;
}
#endif
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 04736212a21c..9bac78e111c6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -1289,6 +1289,21 @@ static const struct switchdev_ops
mlx5e_rep_switchdev_ops = {
.switchdev_port_attr_get= mlx5e_attr_get,
};
+static int mlx5e_rep_swdev_port_attr_event(unsigned long event,
+ struct net_device *dev,
+ struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+ int rc;
+
+ if (event != SWITCHDEV_PORT_ATTR_GET)
+ return NOTIFY_DONE;
+
+ rc = mlx5e_attr_get(dev, port_attr_info->attr);
+ port_attr_info->handled = true;
+
+ return notifier_from_errno(rc);
+}
+
static const struct net_device_ops mlx5e_netdev_ops_vf_rep = {
.ndo_open= mlx5e_vf_rep_open,
.ndo_stop= mlx5e_vf_rep_close,
@@ -1321,6 +1336,22 @@ static const struct net_device_ops
mlx5e_netdev_ops_uplink_rep = {
.ndo_get_vf_stats= mlx5e_get_vf_stats,
};
+static int mlx5e_rep_swdev_blocking_event(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct net_device *netdev = switchdev_notifier_info_to_dev(ptr);
+
+ if (netdev->netdev_ops != _netdev_ops_vf_rep)
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case SWITCHDEV_PORT_ATTR_GET:
+ return mlx5e_rep_swdev_port_attr_event(event, netdev, ptr);
+ }
+
+ return NOTIFY_DONE;
+}
+
bool mlx5e_eswitch_rep(struct net_device *netdev)
{
if (netdev->netdev_ops == _netdev_ops_vf_rep ||
@@ -1798,11 +1829,20 @@ static void *mlx5e_vport_rep_get_proto_dev(struct
mlx5_eswitch_rep *rep)
return rpriv->netdev;
}
-void mlx5e_rep_register_vport_reps(struct mlx5_core_dev *mdev)
+static struct notifier_block mlx5e_rep_swdev_nb = {
+ .notifier_call = mlx5e_rep_swdev_blocking_event,
+};
+
+int mlx5e_rep_register_vport_reps(struct mlx5_core_dev *mdev)
{
struct mlx5_eswitch *esw = mdev->priv.eswitch;
int total_vfs = MLX5_TOTAL_VPORTS(mdev);
int vport;
+ int rc;
+
+ rc = register_switchdev_blocking_notifier(_rep_swdev_nb);
+ if (rc)
+ return rc;
for (vport = 0; vport < total_vfs; vport++) {
struct mlx5_eswitch_rep_if rep_if = {};
@@ -1812,6 +1852,8 @@ void mlx5e_rep_register_vport_reps(struct mlx5_core_dev
*mdev)
rep_if.get_proto_dev = mlx5e_vport_rep_get_proto_dev;
mlx5_eswitch_register_vport_rep(esw, vport, _if, REP_ETH);
}
+
+ return rc;
}
void mlx5e_rep_unregister_vport_reps(struct mlx5_core_dev *mdev)
@@ -1822,4 +1864,5 @@ void mlx5e_rep_unregister_vport_reps(struct mlx5_core_dev
*mdev)
for (vport = total_vfs - 1; vport >= 0; vport--)
mlx5_eswitch_unregister_vport_rep(esw, vport, REP_ETH);
+ unregister_switchdev_blocking_notifier(_rep_swdev_nb);
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
index edd722824697..b9e0507f6100 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
@@ -162,7 +162,7 @@ struct mlx5e_rep_sq {
};
void *mlx5e_alloc_nic_rep_priv(struct mlx5_core_dev *mdev);
-void
[RFC net-next 13/13] net: switchdev: Replace port attr get/set SDO with a notification
Drop switchdev_ops.switchdev_port_attr_get and _set. Drop the uses of
this field from all clients, which were migrated to use switchdev
notification in the previous patches.
Add a new function switchdev_port_attr_notify() that sends the switchdev
notifications SWITCHDEV_PORT_ATTR_GET and _SET.
Update switchdev_port_attr_get() to dispatch to this new function. Drop
__switchdev_port_attr_set() and update switchdev_port_attr_set()
likewise.
Signed-off-by: Florian Fainelli
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 5 -
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c | 5 -
.../net/ethernet/cavium/liquidio/lio_main.c | 5 -
.../net/ethernet/cavium/liquidio/lio_vf_rep.c | 5 -
.../net/ethernet/mellanox/mlx5/core/en_rep.c | 7 --
.../net/ethernet/mellanox/mlxsw/spectrum.c| 12 --
.../net/ethernet/mellanox/mlxsw/spectrum.h| 2 -
.../mellanox/mlxsw/spectrum_switchdev.c | 13 ---
.../net/ethernet/mellanox/mlxsw/switchx2.c| 5 -
drivers/net/ethernet/mscc/ocelot.c| 6 -
.../ethernet/netronome/nfp/nfp_net_common.c | 2 -
.../net/ethernet/netronome/nfp/nfp_net_repr.c | 2 -
drivers/net/ethernet/netronome/nfp/nfp_port.c | 4 -
drivers/net/ethernet/rocker/rocker_main.c | 6 -
drivers/net/netdevsim/netdev.c| 5 -
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 6 -
include/linux/netdevice.h | 3 -
include/net/switchdev.h | 18 ---
net/dsa/slave.c | 6 -
net/switchdev/switchdev.c | 107 ++
20 files changed, 37 insertions(+), 187 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index c3178ca4a004..78d2d76de1a7 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10007,10 +10007,6 @@ static int bnxt_swdev_port_attr_get(struct net_device
*dev,
return bnxt_port_attr_get(netdev_priv(dev), attr);
}
-static const struct switchdev_ops bnxt_switchdev_ops = {
- .switchdev_port_attr_get= bnxt_swdev_port_attr_get
-};
-
static const struct net_device_ops bnxt_netdev_ops = {
.ndo_open = bnxt_open,
.ndo_start_xmit = bnxt_start_xmit,
@@ -10439,7 +10435,6 @@ static int bnxt_init_one(struct pci_dev *pdev, const
struct pci_device_id *ent)
dev->netdev_ops = _netdev_ops;
dev->watchdog_timeo = BNXT_TX_TIMEOUT;
dev->ethtool_ops = _ethtool_ops;
- SWITCHDEV_SET_OPS(dev, _switchdev_ops);
pci_set_drvdata(pdev, dev);
rc = bnxt_alloc_hwrm_resources(bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
index a06f93b49dd5..13db3bf4271f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
@@ -248,10 +248,6 @@ int bnxt_vf_rep_port_attr_get(struct net_device *dev,
return bnxt_port_attr_get(vf_rep->bp, attr);
}
-static const struct switchdev_ops bnxt_vf_rep_switchdev_ops = {
- .switchdev_port_attr_get= bnxt_vf_rep_port_attr_get
-};
-
static const struct ethtool_ops bnxt_vf_rep_ethtool_ops = {
.get_drvinfo= bnxt_vf_rep_get_drvinfo
};
@@ -392,7 +388,6 @@ static void bnxt_vf_rep_netdev_init(struct bnxt *bp, struct
bnxt_vf_rep *vf_rep,
dev->netdev_ops = _vf_rep_netdev_ops;
dev->ethtool_ops = _vf_rep_ethtool_ops;
- SWITCHDEV_SET_OPS(dev, _vf_rep_switchdev_ops);
/* Just inherit all the featues of the parent PF as the VF-R
* uses the RX/TX rings of the parent PF
*/
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c
b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index b9d48e4181fc..19ebf2a3aa49 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -3222,10 +3222,6 @@ lio_pf_switchdev_attr_get(struct net_device *dev, struct
switchdev_attr *attr)
return 0;
}
-static const struct switchdev_ops lio_pf_switchdev_ops = {
- .switchdev_port_attr_get = lio_pf_switchdev_attr_get,
-};
-
static int liquidio_get_vf_stats(struct net_device *netdev, int vfidx,
struct ifla_vf_stats *vf_stats)
{
@@ -3580,7 +3576,6 @@ static int setup_nic_devices(struct octeon_device
*octeon_dev)
* netdev tasks.
*/
netdev->netdev_ops =
- SWITCHDEV_SET_OPS(netdev, _pf_switchdev_ops);
retval = netif_set_real_num_rx_queues(netdev, num_oqueues);
if (retval) {
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c
b/drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c
index d396c004c1be..0aa64ffae8b6 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c
@@
[RFC net-next 05/13] net: mscc: ocelot: Handle SWITCHDEV_PORT_ATTR_GET/SET
Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.
Prepare ocelot to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET/SET and simply translate that into the existing
ocelot_port_attr_{set,get} calls.
Signed-off-by: Florian Fainelli
---
drivers/net/ethernet/mscc/ocelot.c | 24
1 file changed, 24 insertions(+)
diff --git a/drivers/net/ethernet/mscc/ocelot.c
b/drivers/net/ethernet/mscc/ocelot.c
index c6a575eb0ff5..8043347a1ed4 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1589,6 +1589,27 @@ struct notifier_block ocelot_netdevice_nb __read_mostly
= {
};
EXPORT_SYMBOL(ocelot_netdevice_nb);
+static int
+ocelot_switchdev_port_attr_event(unsigned long event,
+ struct net_device *netdev,
+ struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+ int err = -EOPNOTSUPP;
+
+ switch (event) {
+ case SWITCHDEV_PORT_ATTR_SET:
+ err = ocelot_port_attr_set(netdev, port_attr_info->attr,
+ port_attr_info->trans);
+ break;
+ case SWITCHDEV_PORT_ATTR_GET:
+ err = ocelot_port_attr_get(netdev, port_attr_info->attr);
+ break;
+ }
+
+ port_attr_info->handled = true;
+ return notifier_from_errno(err);
+}
+
static int ocelot_switchdev_blocking_event(struct notifier_block *unused,
unsigned long event, void *ptr)
{
@@ -1607,6 +1628,9 @@ static int ocelot_switchdev_blocking_event(struct
notifier_block *unused,
ocelot_netdevice_dev_check,
ocelot_port_obj_del);
return notifier_from_errno(err);
+ case SWITCHDEV_PORT_ATTR_SET:
+ case SWITCHDEV_PORT_ATTR_GET:
+ return ocelot_switchdev_port_attr_event(event, dev, ptr);
}
return NOTIFY_DONE;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC net-next 07/13] bnxt: Handle SWITCHDEV_PORT_ATTR_GET event
Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.
Prepare bnxt to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET and simply translate that into the existing
switchdev_ops::switchdev_port_attr_get operation.
We register a single blocking switchdev notifier for the entire driver
instance and we differentiate a PF from a VF by comparing the network
device's netdev_ops with the ones that this driver manages.
Signed-off-by: Florian Fainelli
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 58 ++-
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c | 4 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h | 8 +++
3 files changed, 67 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 6a512871176b..c3178ca4a004 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10045,6 +10045,45 @@ static const struct net_device_ops bnxt_netdev_ops = {
.ndo_get_phys_port_name = bnxt_get_phys_port_name
};
+static int bnxt_swdev_port_attr_event(unsigned long event,
+ struct net_device *dev,
+ struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+ int rc;
+
+ if (event != SWITCHDEV_PORT_ATTR_GET)
+ return NOTIFY_DONE;
+
+ if (bnxt_dev_is_vf_rep(dev))
+ rc = bnxt_vf_rep_port_attr_get(dev, port_attr_info->attr);
+ else
+ rc = bnxt_port_attr_get(netdev_priv(dev), port_attr_info->attr);
+
+ port_attr_info->handled = true;
+ return notifier_from_errno(rc);
+}
+
+static int bnxt_switchdev_blocking_event(struct notifier_block *nb,
+unsigned long event, void *ptr)
+{
+ struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+
+ if (dev->netdev_ops != _netdev_ops &&
+ !bnxt_dev_is_vf_rep(dev))
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case SWITCHDEV_PORT_ATTR_GET:
+ return bnxt_swdev_port_attr_event(event, dev, ptr);
+ }
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block bnxt_swdev_blocking_nb = {
+ .notifier_call = bnxt_switchdev_blocking_event,
+};
+
static void bnxt_remove_one(struct pci_dev *pdev)
{
struct net_device *dev = pci_get_drvdata(pdev);
@@ -10817,8 +10856,24 @@ static struct pci_driver bnxt_pci_driver = {
static int __init bnxt_init(void)
{
+ int rc;
+
bnxt_debug_init();
- return pci_register_driver(_pci_driver);
+ rc = register_switchdev_blocking_notifier(_swdev_blocking_nb);
+ if (rc)
+ goto err_debug;
+
+ rc = pci_register_driver(_pci_driver);
+ if (rc)
+ goto err_unreg_notifier;
+
+ return rc;
+
+err_unreg_notifier:
+ unregister_switchdev_notifier(_swdev_blocking_nb);
+err_debug:
+ bnxt_debug_exit();
+ return rc;
}
static void __exit bnxt_exit(void)
@@ -10826,6 +10881,7 @@ static void __exit bnxt_exit(void)
pci_unregister_driver(_pci_driver);
if (bnxt_pf_wq)
destroy_workqueue(bnxt_pf_wq);
+ unregister_switchdev_notifier(_swdev_blocking_nb);
bnxt_debug_exit();
}
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
index 9a25c05aa571..a06f93b49dd5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
@@ -237,8 +237,8 @@ static void bnxt_vf_rep_get_drvinfo(struct net_device *dev,
strlcpy(info->version, DRV_MODULE_VERSION, sizeof(info->version));
}
-static int bnxt_vf_rep_port_attr_get(struct net_device *dev,
-struct switchdev_attr *attr)
+int bnxt_vf_rep_port_attr_get(struct net_device *dev,
+ struct switchdev_attr *attr)
{
struct bnxt_vf_rep *vf_rep = netdev_priv(dev);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h
b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h
index d7287651422f..ba24ac222182 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h
@@ -32,6 +32,8 @@ bool bnxt_dev_is_vf_rep(struct net_device *dev);
int bnxt_dl_eswitch_mode_get(struct devlink *devlink, u16 *mode);
int bnxt_dl_eswitch_mode_set(struct devlink *devlink, u16 mode,
struct netlink_ext_ack *extack);
+int bnxt_vf_rep_port_attr_get(struct net_device *dev,
+ struct switchdev_attr *attr);
#else
@@ -61,5 +63,11 @@ static inline bool bnxt_dev_is_vf_rep(struct net_device *dev)
{
return false;
}
+
+static inline int bnxt_vf_rep_port_attr_get(struct net_device *dev,
+ struct switchdev_attr *attr)
+{
[RFC net-next 03/13] net: dsa: Handle SWITCHDEV_PORT_ATTR_GET/SET
Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.
Prepare DSA to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET/SET and simply translate that into the existing
dsa_slave_port_attr_{set,get} calls.
Signed-off-by: Florian Fainelli
---
net/dsa/slave.c | 24
1 file changed, 24 insertions(+)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 91de3a663226..bc7d5092a1c7 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1549,6 +1549,27 @@ dsa_slave_switchdev_port_obj_event(unsigned long event,
return notifier_from_errno(err);
}
+static int
+dsa_slave_switchdev_port_attr_event(unsigned long event,
+ struct net_device *netdev,
+ struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+ int err = -EOPNOTSUPP;
+
+ switch (event) {
+ case SWITCHDEV_PORT_ATTR_SET:
+ err = dsa_slave_port_attr_set(netdev, port_attr_info->attr,
+ port_attr_info->trans);
+ break;
+ case SWITCHDEV_PORT_ATTR_GET:
+ err = dsa_slave_port_attr_get(netdev, port_attr_info->attr);
+ break;
+ }
+
+ port_attr_info->handled = true;
+ return notifier_from_errno(err);
+}
+
static int dsa_slave_switchdev_blocking_event(struct notifier_block *unused,
unsigned long event, void *ptr)
{
@@ -1561,6 +1582,9 @@ static int dsa_slave_switchdev_blocking_event(struct
notifier_block *unused,
case SWITCHDEV_PORT_OBJ_ADD: /* fall through */
case SWITCHDEV_PORT_OBJ_DEL:
return dsa_slave_switchdev_port_obj_event(event, dev, ptr);
+ case SWITCHDEV_PORT_ATTR_SET: /* fallthrough */
+ case SWITCHDEV_PORT_ATTR_GET:
+ return dsa_slave_switchdev_port_attr_event(event, dev, ptr);
}
return NOTIFY_DONE;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC net-next 01/13] switchdev: Add SWITCHDEV_PORT_ATTR_SET, SWITCHDEV_PORT_ATTR_GET
In preparation for allowing switchdev enabled drivers to veto specific
attribute settings from within the context of the caller, introduce a
new switchdev notifier type for port attributes.
Suggested-by: Ido Schimmel
Signed-off-by: Florian Fainelli
---
include/net/switchdev.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 63843ae5dc81..e62fb2693e00 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -145,6 +145,9 @@ enum switchdev_notifier_type {
SWITCHDEV_VXLAN_FDB_ADD_TO_DEVICE,
SWITCHDEV_VXLAN_FDB_DEL_TO_DEVICE,
SWITCHDEV_VXLAN_FDB_OFFLOADED,
+
+ SWITCHDEV_PORT_ATTR_SET, /* Blocking. */
+ SWITCHDEV_PORT_ATTR_GET, /* Blocking. */
};
struct switchdev_notifier_info {
@@ -167,6 +170,13 @@ struct switchdev_notifier_port_obj_info {
bool handled;
};
+struct switchdev_notifier_port_attr_info {
+ struct switchdev_notifier_info info; /* must be first */
+ struct switchdev_attr *attr;
+ struct switchdev_trans *trans;
+ bool handled;
+};
+
static inline struct net_device *
switchdev_notifier_info_to_dev(const struct switchdev_notifier_info *info)
{
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC net-next 02/13] rocker: Handle SWITCHDEV_PORT_ATTR_GET/SET
Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.
Prepare rocker to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET/SET and simply translate that into the existing
rocker_port_attr_{set,get} calls.
Signed-off-by: Florian Fainelli
---
drivers/net/ethernet/rocker/rocker_main.c | 24 +++
1 file changed, 24 insertions(+)
diff --git a/drivers/net/ethernet/rocker/rocker_main.c
b/drivers/net/ethernet/rocker/rocker_main.c
index 62a205eba9f7..f25f7b4345b8 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2827,6 +2827,27 @@ rocker_switchdev_port_obj_event(unsigned long event,
struct net_device *netdev,
return notifier_from_errno(err);
}
+static int
+rocker_switchdev_port_attr_event(unsigned long event, struct net_device
*netdev,
+struct switchdev_notifier_port_attr_info
+*port_attr_info)
+{
+ int err = -EOPNOTSUPP;
+
+ switch (event) {
+ case SWITCHDEV_PORT_ATTR_SET:
+ err = rocker_port_attr_set(netdev, port_attr_info->attr,
+ port_attr_info->trans);
+ break;
+ case SWITCHDEV_PORT_ATTR_GET:
+ err = rocker_port_attr_get(netdev, port_attr_info->attr);
+ break;
+ }
+
+ port_attr_info->handled = true;
+ return notifier_from_errno(err);
+}
+
static int rocker_switchdev_blocking_event(struct notifier_block *unused,
unsigned long event, void *ptr)
{
@@ -2839,6 +2860,9 @@ static int rocker_switchdev_blocking_event(struct
notifier_block *unused,
case SWITCHDEV_PORT_OBJ_ADD:
case SWITCHDEV_PORT_OBJ_DEL:
return rocker_switchdev_port_obj_event(event, dev, ptr);
+ case SWITCHDEV_PORT_ATTR_SET:
+ case SWITCHDEV_PORT_ATTR_GET:
+ return rocker_switchdev_port_attr_event(event, dev, ptr);
}
return NOTIFY_DONE;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC net-next 00/13] Get rid of switchdev_ops
Hi all,
This patch series converts SWITCHDEV_PORT_ATTR_{GET,SET} to use a
blocking notifier, similar to how SWITCHDEV_PORT_OBJ_{ADD,DEL} has been
changed recently by Petr.
This was suggested by Ido to help with a particular use case I have
where I want to be able to veto a switchdev bridge attribute from a
driver (multicast_snooping).
Please review since I may not have gotten the driver abstraction right,
especially for mlx5e and nfp since these are *hum* *hum* large drivers.
Florian Fainelli (13):
switchdev: Add SWITCHDEV_PORT_ATTR_SET, SWITCHDEV_PORT_ATTR_GET
rocker: Handle SWITCHDEV_PORT_ATTR_GET/SET
net: dsa: Handle SWITCHDEV_PORT_ATTR_GET/SET
mlxsw: spectrum_switchdev: Handle SWITCHDEV_PORT_ATTR_GET/SET
net: mscc: ocelot: Handle SWITCHDEV_PORT_ATTR_GET/SET
staging: fsl-dpaa2: ethsw: Handle SWITCHDEV_PORT_ATTR_GET/SET
bnxt: Handle SWITCHDEV_PORT_ATTR_GET event
liquidio: Handle SWITCHDEV_PORT_ATTR_GET event
mlxsw: switchx2: Handle SWITCHDEV_PORT_ATTR_GET event
net/mlx5e: Handle SWITCHDEV_PORT_ATTR_GET event
nfp: Handle SWITCHDEV_PORT_ATTR_GET event
netdevsim: Handle SWITCHDEV_PORT_ATTR_GET event
net: switchdev: Replace port attr get/set SDO with a notification
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 63 ++-
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c | 9 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h | 8 ++
.../net/ethernet/cavium/liquidio/lio_main.c | 53 -
.../net/ethernet/cavium/liquidio/lio_vf_rep.c | 48 +++-
.../net/ethernet/mellanox/mlx5/core/en_main.c | 4 +-
.../net/ethernet/mellanox/mlx5/core/en_rep.c | 50 ++--
.../net/ethernet/mellanox/mlx5/core/en_rep.h | 2 +-
.../net/ethernet/mellanox/mlxsw/spectrum.c| 12 --
.../net/ethernet/mellanox/mlxsw/spectrum.h| 2 -
.../mellanox/mlxsw/spectrum_switchdev.c | 36 +++---
.../net/ethernet/mellanox/mlxsw/switchx2.c| 46 +++-
drivers/net/ethernet/mscc/ocelot.c| 30 -
drivers/net/ethernet/netronome/nfp/nfp_main.c | 14 ++-
drivers/net/ethernet/netronome/nfp/nfp_net.h | 3 +
.../ethernet/netronome/nfp/nfp_net_common.c | 19 +++-
.../net/ethernet/netronome/nfp/nfp_net_repr.c | 2 -
drivers/net/ethernet/netronome/nfp/nfp_port.c | 17 ++-
drivers/net/ethernet/netronome/nfp/nfp_port.h | 5 +-
drivers/net/ethernet/rocker/rocker_main.c | 30 -
drivers/net/netdevsim/netdev.c| 46 +++-
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 30 -
include/linux/netdevice.h | 3 -
include/net/switchdev.h | 28 ++---
net/dsa/slave.c | 30 -
net/switchdev/switchdev.c | 107 ++
26 files changed, 503 insertions(+), 194 deletions(-)
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/2] staging: iio: frequency: ad9833: Load clock using clock framework
From: Beniamin Bia
The clock frequency is loaded from device-tree using clock framework
instead of statically value. The change allow configuration of
the device via device-trees and better initialization sequence.
This is part of broader effort to add device-tree support to this driver
and take it out from staging.
Signed-off-by: Beniamin Bia
---
Changes in v2:
-the intermidiate clk variable was replaced by the variable in
device state
-st variable may be uninitialized warning was fixed by adding a new
error label
drivers/staging/iio/frequency/ad9834.c | 35 ++
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/iio/frequency/ad9834.c
b/drivers/staging/iio/frequency/ad9834.c
index f4f5eaa15e30..f036f75d1f22 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -6,6 +6,7 @@
* Licensed under the GPL-2.
*/
+#include
#include
#include
#include
@@ -71,7 +72,7 @@
struct ad9834_state {
struct spi_device *spi;
struct regulator*reg;
- unsigned intmclk;
+ struct clk *mclk;
unsigned short control;
unsigned short devid;
struct spi_transfer xfer;
@@ -110,12 +111,15 @@ static unsigned int ad9834_calc_freqreg(unsigned long
mclk, unsigned long fout)
static int ad9834_write_frequency(struct ad9834_state *st,
unsigned long addr, unsigned long fout)
{
+ unsigned long clk_freq;
unsigned long regval;
- if (fout > (st->mclk / 2))
+ clk_freq = clk_get_rate(st->mclk);
+
+ if (fout > (clk_freq / 2))
return -EINVAL;
- regval = ad9834_calc_freqreg(st->mclk, fout);
+ regval = ad9834_calc_freqreg(clk_freq, fout);
st->freq_data[0] = cpu_to_be16(addr | (regval &
RES_MASK(AD9834_FREQ_BITS / 2)));
@@ -415,7 +419,14 @@ static int ad9834_probe(struct spi_device *spi)
spi_set_drvdata(spi, indio_dev);
st = iio_priv(indio_dev);
mutex_init(>lock);
- st->mclk = 2500;
+ st->mclk = devm_clk_get(>dev, NULL);
+
+ ret = clk_prepare_enable(st->mclk);
+ if (ret) {
+ dev_err(>dev, "Failed to enable master clock\n");
+ goto error_disable_reg;
+ }
+
st->spi = spi;
st->devid = spi_get_device_id(spi)->driver_data;
st->reg = reg;
@@ -460,31 +471,32 @@ static int ad9834_probe(struct spi_device *spi)
ret = spi_sync(st->spi, >msg);
if (ret) {
dev_err(>dev, "device init failed\n");
- goto error_disable_reg;
+ goto error_clock_unprepare;
}
ret = ad9834_write_frequency(st, AD9834_REG_FREQ0, 100);
if (ret)
- goto error_disable_reg;
+ goto error_clock_unprepare;
ret = ad9834_write_frequency(st, AD9834_REG_FREQ1, 500);
if (ret)
- goto error_disable_reg;
+ goto error_clock_unprepare;
ret = ad9834_write_phase(st, AD9834_REG_PHASE0, 512);
if (ret)
- goto error_disable_reg;
+ goto error_clock_unprepare;
ret = ad9834_write_phase(st, AD9834_REG_PHASE1, 1024);
if (ret)
- goto error_disable_reg;
+ goto error_clock_unprepare;
ret = iio_device_register(indio_dev);
if (ret)
- goto error_disable_reg;
+ goto error_clock_unprepare;
return 0;
-
+error_clock_unprepare:
+ clk_disable_unprepare(st->mclk);
error_disable_reg:
regulator_disable(reg);
@@ -497,6 +509,7 @@ static int ad9834_remove(struct spi_device *spi)
struct ad9834_state *st = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
+ clk_disable_unprepare(st->mclk);
regulator_disable(st->reg);
return 0;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] media: staging/intel-ipu3: Implement lock for stream on/off operations
Hi Sakari, > Subject: Re: [PATCH] media: staging/intel-ipu3: Implement lock for stream > on/off operations > > Hi Raj, > > On Wed, Jan 30, 2019 at 05:17:15PM +, Mani, Rajmohan wrote: > > Hi Sakari, > > > > > -Original Message- > > > From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com] > > > Sent: Wednesday, January 30, 2019 12:59 AM > > > To: Mani, Rajmohan > > > Cc: Mauro Carvalho Chehab ; Greg Kroah-Hartman > > > ; linux-me...@vger.kernel.org; > > > de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org; Laurent > > > Pinchart ; Jacopo Mondi > > > ; Qiu, Tian Shu ; Cao, > > > Bingbu ; z...@paasikivi.fi.intel.com; Zhi, Yong > > > ; hverk...@xs4all.nl; tf...@chromium.org > > > Subject: Re: [PATCH] media: staging/intel-ipu3: Implement lock for > > > stream on/off operations > > > > > > Hi Rajmohan, > > > > > > On Tue, Jan 29, 2019 at 02:27:36PM -0800, Rajmohan Mani wrote: > > > > Currently concurrent stream off operations on ImgU nodes are not > > > > synchronized, leading to use-after-free bugs (as reported by KASAN). > > > > > > > > [ 250.090724] BUG: KASAN: use-after-free in > > > > ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu] [ 250.090726] Read of > > > > size 8 at addr 888127b29bc0 by task yavta/18836 [ 250.090731] > > > > Hardware > > > > name: HP Soraka/Soraka, BIOS Google_Soraka.10431.17.0 03/22/2018 [ > > > 250.090732] Call Trace: > > > > [ 250.090735] dump_stack+0x6a/0xb1 [ 250.090739] > > > > print_address_description+0x8e/0x279 > > > > [ 250.090743] ? ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu] [ > > > > 250.090746] kasan_report+0x260/0x28a [ 250.090750] > > > > ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu] [ 250.090754] > > > > ipu3_css_pool_cleanup+0x24/0x37 [ipu3_imgu] [ 250.090759] > > > > ipu3_css_pipeline_cleanup+0x61/0xb9 [ipu3_imgu] [ 250.090763] > > > > ipu3_css_stop_streaming+0x1f2/0x321 [ipu3_imgu] [ 250.090768] > > > > imgu_s_stream+0x94/0x443 [ipu3_imgu] [ 250.090772] ? > > > > ipu3_vb2_buf_queue+0x280/0x280 [ipu3_imgu] [ 250.090775] ? > > > > vb2_dma_sg_unmap_dmabuf+0x16/0x6f [videobuf2_dma_sg] [ > > > > 250.090778] > > > ? > > > > vb2_buffer_in_use+0x36/0x58 [videobuf2_common] [ 250.090782] > > > > ipu3_vb2_stop_streaming+0xf9/0x135 [ipu3_imgu] > > > > > > > > Implemented a lock to synchronize imgu stream on / off operations > > > > and the modification of streaming flag (in struct imgu_device), to > > > > prevent these issues. > > > > > > > > Reported-by: Laurent Pinchart > > > > Suggested-by: Laurent Pinchart > > > > > > > > Signed-off-by: Rajmohan Mani > > > > --- > > > > drivers/staging/media/ipu3/ipu3-v4l2.c | 6 ++ > > > > drivers/staging/media/ipu3/ipu3.c | 3 +++ > > > > drivers/staging/media/ipu3/ipu3.h | 4 > > > > 3 files changed, 13 insertions(+) > > > > > > > > diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c > > > > b/drivers/staging/media/ipu3/ipu3-v4l2.c > > > > index c7936032beb9..cf7e917cd0c8 100644 > > > > --- a/drivers/staging/media/ipu3/ipu3-v4l2.c > > > > +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c > > > > @@ -507,12 +507,15 @@ static int ipu3_vb2_start_streaming(struct > > > vb2_queue *vq, unsigned int count) > > > > goto fail_stop_pipeline; > > > > } > > > > > > > > + mutex_lock(>streaming_lock); > > > > + > > > > > > You appear to be using imgu_device.lock (while searching buffers to > > > queue to the device) as well as imgu_video_device.lock (qbuf, dqbuf) > > > to serialise access to imgu_video_device.buffers list. > > > > Ack > > > > > The two locks may be acquired at the same time but each by different > > > processes. That needs to be addressed, but probably not in this > > > patch. > > > > > > > The node specific locks will be used by different processes and all of > > these processes will be competing commonly (and successfully) for the > imgu_device lock. > > I will look into this more. > > > > > I wonder if it'd be more simple to use imgu->lock here instead of > > > adding a new one. > > > > > > > Extending imgu->lock here, does not work in this case, as > > imgu_queue_buffers() will be stuck acquiring imgu->lock, which was > > already acquired by imgu_s_stream() through ipu3_vb2_start_streaming(). > > You could move acquiring the lock out of these functions. It would also seem > that there is device-wide streaming state etc. information to which the access > should also be serialised. Currently it's relying on the node-specific lock > only > which does not help. > Ack. Let me look into this more. > Can you grab the lock right after dev_dbg() line in the function? > In order to reduce the amount of code that's run with the lock held, I placed the lock here. Do you see issues in calls to imgu_sd subdevs and media_pipeline_start(), without the lock being held? > The lock should be also acquired before testing imgu->streaming in > ipu3_vb2_buf_queue() to make sure it won't change in the meantime. > I thought about this and decided against this
[PATCH v2 1/2] staging: iio: frequency: ad9833: Get frequency value statically
From: Beniamin Bia
The values from platform data were replaced by statically values.
This was just a intermediate step of taking this driver out of staging and
load data from device tree.
Signed-off-by: Beniamin Bia
---
Changes in v2:
- The platform data structure was removed and the values are
written directly
drivers/staging/iio/frequency/ad9834.c | 21 +++
drivers/staging/iio/frequency/ad9834.h | 28 --
2 files changed, 7 insertions(+), 42 deletions(-)
diff --git a/drivers/staging/iio/frequency/ad9834.c
b/drivers/staging/iio/frequency/ad9834.c
index 995acdd7c942..f4f5eaa15e30 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -391,16 +391,11 @@ static const struct iio_info ad9833_info = {
static int ad9834_probe(struct spi_device *spi)
{
- struct ad9834_platform_data *pdata = dev_get_platdata(>dev);
struct ad9834_state *st;
struct iio_dev *indio_dev;
struct regulator *reg;
int ret;
- if (!pdata) {
- dev_dbg(>dev, "no platform data?\n");
- return -ENODEV;
- }
reg = devm_regulator_get(>dev, "avdd");
if (IS_ERR(reg))
@@ -420,7 +415,7 @@ static int ad9834_probe(struct spi_device *spi)
spi_set_drvdata(spi, indio_dev);
st = iio_priv(indio_dev);
mutex_init(>lock);
- st->mclk = pdata->mclk;
+ st->mclk = 2500;
st->spi = spi;
st->devid = spi_get_device_id(spi)->driver_data;
st->reg = reg;
@@ -456,11 +451,9 @@ static int ad9834_probe(struct spi_device *spi)
spi_message_add_tail(>freq_xfer[1], >freq_msg);
st->control = AD9834_B28 | AD9834_RESET;
+ st->control |= AD9834_DIV2;
- if (!pdata->en_div2)
- st->control |= AD9834_DIV2;
-
- if (!pdata->en_signbit_msb_out && (st->devid == ID_AD9834))
+ if (st->devid == ID_AD9834)
st->control |= AD9834_SIGN_PIB;
st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
@@ -470,19 +463,19 @@ static int ad9834_probe(struct spi_device *spi)
goto error_disable_reg;
}
- ret = ad9834_write_frequency(st, AD9834_REG_FREQ0, pdata->freq0);
+ ret = ad9834_write_frequency(st, AD9834_REG_FREQ0, 100);
if (ret)
goto error_disable_reg;
- ret = ad9834_write_frequency(st, AD9834_REG_FREQ1, pdata->freq1);
+ ret = ad9834_write_frequency(st, AD9834_REG_FREQ1, 500);
if (ret)
goto error_disable_reg;
- ret = ad9834_write_phase(st, AD9834_REG_PHASE0, pdata->phase0);
+ ret = ad9834_write_phase(st, AD9834_REG_PHASE0, 512);
if (ret)
goto error_disable_reg;
- ret = ad9834_write_phase(st, AD9834_REG_PHASE1, pdata->phase1);
+ ret = ad9834_write_phase(st, AD9834_REG_PHASE1, 1024);
if (ret)
goto error_disable_reg;
diff --git a/drivers/staging/iio/frequency/ad9834.h
b/drivers/staging/iio/frequency/ad9834.h
index ae620f38eb49..da7e83ceedad 100644
--- a/drivers/staging/iio/frequency/ad9834.h
+++ b/drivers/staging/iio/frequency/ad9834.h
@@ -8,32 +8,4 @@
#ifndef IIO_DDS_AD9834_H_
#define IIO_DDS_AD9834_H_
-/*
- * TODO: struct ad7887_platform_data needs to go into include/linux/iio
- */
-
-/**
- * struct ad9834_platform_data - platform specific information
- * @mclk: master clock in Hz
- * @freq0: power up freq0 tuning word in Hz
- * @freq1: power up freq1 tuning word in Hz
- * @phase0:power up phase0 value [0..4095] correlates with 0..2PI
- * @phase1:power up phase1 value [0..4095] correlates with 0..2PI
- * @en_div2: digital output/2 is passed to the SIGN BIT OUT pin
- * @en_signbit_msb_out:the MSB (or MSB/2) of the DAC data is connected
to the
- * SIGN BIT OUT pin. en_div2 controls whether it is the MSB
- * or MSB/2 that is output. if en_signbit_msb_out=false,
- * the on-board comparator is connected to SIGN BIT OUT
- */
-
-struct ad9834_platform_data {
- unsigned intmclk;
- unsigned intfreq0;
- unsigned intfreq1;
- unsigned short phase0;
- unsigned short phase1;
- boolen_div2;
- boolen_signbit_msb_out;
-};
-
#endif /* IIO_DDS_AD9834_H_ */
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions
> From: Kimberly Brown > Sent: Thursday, January 31, 2019 9:47 AM > ... > 2) Prevent a deadlock that can occur between the proposed mutex_lock() > call in the vmbus_chan_attr_show() function and the sysfs/kernfs functions. Hi Kim, Can you please share more details about the deadlock? It's unclear to me how the deadlock happens. > I've identified two possible solutions to the deadlock: > > 1) Add a new mutex to the vmbus_channel struct which protects changes to > "channel->state". Use this new mutex in vmbus_chan_attr_show() instead of > "vmbus_connection.channel_mutex". > > 2) Use "vmbus_connection.channel_mutex" in vmbus_chan_attr_show() as > originally proposed, and acquire it with mutex_trylock(). If the mutex > cannot be acquired, return -EINVAL. It looks more like a workaround. IMO we should try to find a real fix. :-) > I'm leaning towards (2), using mutex_trylock(). > "vmbus_connection.channel_mutex" > appears to be used only when a channel is being opened or closed, so > vmbus_chan_attr_show() should be able to acquire the mutex when the > channel is in use. > > If anyone has suggestions, please let me know. > > Thanks, > Kim Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[driver-core:driver-core-testing 34/36] ERROR: "__arm_smccc_hvc" [drivers/firmware/stratix10-svc.ko] undefined!
tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git driver-core-testing head: 8d84b18f5678d3adfdb9375dfb0d968da2dc753d commit: 095ff29d2b882847f608cf7192361dedc22c0623 [34/36] firmware: intel_stratix10_service: add hardware dependency config: x86_64-allmodconfig (attached as .config) compiler: gcc-8 (Debian 8.2.0-14) 8.2.0 reproduce: git checkout 095ff29d2b882847f608cf7192361dedc22c0623 # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): >> ERROR: "__arm_smccc_hvc" [drivers/firmware/stratix10-svc.ko] undefined! >> ERROR: "__arm_smccc_smc" [drivers/firmware/stratix10-svc.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] staging: iio: ad7780: add gain & filter gpio support
On 02/01, Popa, Stefan Serban wrote: Adding support for gpios, normally doesn't have anything to do with the information stored in the iio_chan_spec struct. Also, we need to try to work with the already defined attributes. In your case, setting the gain GPIO is equivalent to modifying the scale, while setting the filter GPIO results in a change in sampling frequency. Therefore, we need use a macro which has the IIO_CHAN_INFO_SCALE and IIO_CHAN_INFO_FREQUENCY flags set. I think AD_SD_CHANNEL should do just fine. I hope this answers your question. -Stefan It does. Thank you. :) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] nfit: add Hyper-V NVDIMM DSM command set to white list
On Fri, Feb 1, 2019 at 9:14 AM Dexuan Cui wrote: > > > From: Dan Williams > > Sent: Tuesday, January 29, 2019 10:24 PM > > On Mon, Jan 28, 2019 at 4:56 PM Dexuan Cui wrote: > > > > > > > > > Add the Hyper-V _DSM command set to the white list of NVDIMM command > > > sets. > > > > > > Thanks Dan Williams for writing the > > > comment change. > > > --- > > > Changes in v2: > > > Updated the comment and changelog (Thanks, Dan!) > > > Rebased to the tag libnvdimm-fixes-5.0-rc4 of the nvdimm tree. > > > > Thanks for the re-spin, applied. > > Hi Dan, > Unluckily it looks this commit causes a regression on > https://git.kernel.org/pub/scm/linux/kernel/git/djbw/nvdimm.git/log/?h=libnvdimm-pending > > With the patch, "ndctl list" shows nothing, and /dev/pmem0 can't appear. > If I revert the patch, it will be back to normal. > > I attached the config/logs. In the bad case, "dmesg" shows a line > [5.259017] nd_pmem namespace0.0: 0x, too small must be at > least 0x1000 > > Any idea why this happens? I'm digging into the details and I appreciate your > insights. Looks like it is working as expected. The regression you are seeing is the fact that the patch enables the kernel to enable nvdimm-namespace-label reads. Those reads find a namespace index block and a label. Unfortunately the label has the LOCAL flag set and Linux explicitly ignores pmem namespace labels with that bit set. The reason for that is due to the fact that the original definition of the LOCAL bit from v1.1 of the namespace label implementation [1] explicitly limited the LOCAL flag to "block aperture" regions. If you clear that LOCAL flag I expect it will work. To my knowledge Windows pretends that the v1.1 definition never existed. The UEFI 2.7 specification for v1.2 labels states that setting the LOCAL flag is optional when "nlabel", number of labels in the set, is 1. Linux makes that mandatory as LOCAL is redundant when nlabel is 1. That said, the Robustness Principle makes a case that Linux should tolerate the bit being set. However, it's just a non-trivial amount of work to unwind the ingrained block-aperture assumptions of that bit. [1]: http://pmem.io/documents/NVDIMM_Namespace_Spec.pdf ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: staging/intel-ipu3: Implement lock for stream on/off operations
Hi Raj, On Wed, Jan 30, 2019 at 05:17:15PM +, Mani, Rajmohan wrote: > Hi Sakari, > > > -Original Message- > > From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com] > > Sent: Wednesday, January 30, 2019 12:59 AM > > To: Mani, Rajmohan > > Cc: Mauro Carvalho Chehab ; Greg Kroah-Hartman > > ; linux-me...@vger.kernel.org; > > de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org; Laurent Pinchart > > ; Jacopo Mondi ; > > Qiu, Tian Shu ; Cao, Bingbu > > ; z...@paasikivi.fi.intel.com; Zhi, Yong > > ; hverk...@xs4all.nl; tf...@chromium.org > > Subject: Re: [PATCH] media: staging/intel-ipu3: Implement lock for stream > > on/off operations > > > > Hi Rajmohan, > > > > On Tue, Jan 29, 2019 at 02:27:36PM -0800, Rajmohan Mani wrote: > > > Currently concurrent stream off operations on ImgU nodes are not > > > synchronized, leading to use-after-free bugs (as reported by KASAN). > > > > > > [ 250.090724] BUG: KASAN: use-after-free in > > > ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu] [ 250.090726] Read of size 8 > > > at addr 888127b29bc0 by task yavta/18836 [ 250.090731] Hardware > > > name: HP Soraka/Soraka, BIOS Google_Soraka.10431.17.0 03/22/2018 [ > > 250.090732] Call Trace: > > > [ 250.090735] dump_stack+0x6a/0xb1 > > > [ 250.090739] print_address_description+0x8e/0x279 > > > [ 250.090743] ? ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu] [ > > > 250.090746] kasan_report+0x260/0x28a [ 250.090750] > > > ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu] [ 250.090754] > > > ipu3_css_pool_cleanup+0x24/0x37 [ipu3_imgu] [ 250.090759] > > > ipu3_css_pipeline_cleanup+0x61/0xb9 [ipu3_imgu] [ 250.090763] > > > ipu3_css_stop_streaming+0x1f2/0x321 [ipu3_imgu] [ 250.090768] > > > imgu_s_stream+0x94/0x443 [ipu3_imgu] [ 250.090772] ? > > > ipu3_vb2_buf_queue+0x280/0x280 [ipu3_imgu] [ 250.090775] ? > > > vb2_dma_sg_unmap_dmabuf+0x16/0x6f [videobuf2_dma_sg] [ 250.090778] > > ? > > > vb2_buffer_in_use+0x36/0x58 [videobuf2_common] [ 250.090782] > > > ipu3_vb2_stop_streaming+0xf9/0x135 [ipu3_imgu] > > > > > > Implemented a lock to synchronize imgu stream on / off operations and > > > the modification of streaming flag (in struct imgu_device), to prevent > > > these issues. > > > > > > Reported-by: Laurent Pinchart > > > Suggested-by: Laurent Pinchart > > > > > > Signed-off-by: Rajmohan Mani > > > --- > > > drivers/staging/media/ipu3/ipu3-v4l2.c | 6 ++ > > > drivers/staging/media/ipu3/ipu3.c | 3 +++ > > > drivers/staging/media/ipu3/ipu3.h | 4 > > > 3 files changed, 13 insertions(+) > > > > > > diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c > > > b/drivers/staging/media/ipu3/ipu3-v4l2.c > > > index c7936032beb9..cf7e917cd0c8 100644 > > > --- a/drivers/staging/media/ipu3/ipu3-v4l2.c > > > +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c > > > @@ -507,12 +507,15 @@ static int ipu3_vb2_start_streaming(struct > > vb2_queue *vq, unsigned int count) > > > goto fail_stop_pipeline; > > > } > > > > > > + mutex_lock(>streaming_lock); > > > + > > > > You appear to be using imgu_device.lock (while searching buffers to queue to > > the device) as well as imgu_video_device.lock (qbuf, dqbuf) to serialise > > access > > to imgu_video_device.buffers list. > > Ack > > > The two locks may be acquired at the same > > time but each by different processes. That needs to be addressed, but > > probably not in this patch. > > > > The node specific locks will be used by different processes and all of these > processes > will be competing commonly (and successfully) for the imgu_device lock. > I will look into this more. > > > I wonder if it'd be more simple to use imgu->lock here instead of adding a > > new > > one. > > > > Extending imgu->lock here, does not work in this case, as imgu_queue_buffers() > will be stuck acquiring imgu->lock, which was already acquired by > imgu_s_stream() > through ipu3_vb2_start_streaming(). You could move acquiring the lock out of these functions. It would also seem that there is device-wide streaming state etc. information to which the access should also be serialised. Currently it's relying on the node-specific lock only which does not help. Can you grab the lock right after dev_dbg() line in the function? The lock should be also acquired before testing imgu->streaming in ipu3_vb2_buf_queue() to make sure it won't change in the meantime. > > > > /* Start streaming of the whole pipeline now */ > > > dev_dbg(dev, "IMGU streaming is ready to start"); > > > r = imgu_s_stream(imgu, true); > > > if (!r) > > > imgu->streaming = true; > > > > > > + mutex_unlock(>streaming_lock); > > > return 0; > > > > > > fail_stop_pipeline: > > > @@ -543,6 +546,8 @@ static void ipu3_vb2_stop_streaming(struct > > vb2_queue *vq) > > > dev_err(>pci_dev->dev, > > > "failed to stop subdev streaming\n"); > > > > > > + mutex_lock(>streaming_lock); > > > + > > > /* Was this the first node with streaming disabled? */ >
Re: [PATCH v2 1/2] staging: iio: ad7780: add gain & filter gpio support
On Vi, 2019-02-01 at 12:55 -0200, Renato Lui Geh wrote:
> On 01/30, Popa, Stefan Serban wrote:
> >
> > On Du, 2019-01-27 at 18:30 -0200, Renato Lui Geh wrote:
> > >
> > > Previously, the AD7780 driver only supported gpio for the 'powerdown'
> > > pin. This commit adds suppport for the 'gain' and 'filter' pin.
> > >
> > > Signed-off-by: Renato Lui Geh
> > > Signed-off-by: Giuliano Belinassi
> > > Co-developed-by: Giuliano Belinassi
> > > ---
> > > Changes in v2:
> > > - Filter reading changed to mHz
> > > - Storing filter, gain and voltage to chip_infoHi,
> Hi Stefan,
>
> Thanks for the review. Comments inline.
>
> Renato
> >
> >
> > Comments inline.
> >
> > Regards,
> > Stefan
> >
> > >
> > >
> > > drivers/staging/iio/adc/ad7780.c | 103 ++-
> > > --
> > > include/linux/iio/adc/ad_sigma_delta.h | 5 ++
> > > 2 files changed, 99 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/staging/iio/adc/ad7780.c
> > > b/drivers/staging/iio/adc/ad7780.c
> > > index c4a85789c2db..82394e31b168 100644
> > > --- a/drivers/staging/iio/adc/ad7780.c
> > > +++ b/drivers/staging/iio/adc/ad7780.c
> > > @@ -39,6 +39,15 @@
> > > #define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2)
> > > #define AD7170_PATTERN_MASK(AD7780_PAT0 | AD7780_PAT1 |
> > > AD7170_PAT2)
> > >
> > > +#define AD7780_GAIN_GPIO 0
> > > +#define AD7780_FILTER_GPIO 1
> > > +
> > > +#define AD7780_GAIN_MIDPOINT 64
> > > +#define AD7780_FILTER_MIDPOINT 13350
> > > +
> > > +static const unsigned int ad778x_gain[2]= { 1, 128 };
> > > +static const unsigned int ad778x_filter[2] = { 1, 16700 };
> > I would name this array ad778x_odr_avail or something like that.
> Sure
> >
> > We should also consider adding the option to read the available
> > sampling frequencies from user space, but let's leave this for another
> > commit.
> Do you mean returning 10 and 16.7 Hz?
>
> Should this be done before sending the driver to the main tree, or is it
> ok to do something like that after it has left staging?
I think, we should leave it for a future commit.
> >
> >
> > >
> > > +
> > > struct ad7780_chip_info {
> > > struct iio_chan_specchannel;
> > > unsigned intpattern_mask;
> > > @@ -50,7 +59,11 @@ struct ad7780_state {
> > > const struct ad7780_chip_info *chip_info;
> > > struct regulator*reg;
> > > struct gpio_desc*powerdown_gpio;
> > > - unsigned intgain;
> > > + struct gpio_desc*gain_gpio;
> > > + struct gpio_desc*filter_gpio;
> > > + unsigned intgain;
> > > + unsigned intfilter;
> > Also, this could be renamed as odr.
> >
> > >
> > > + unsigned intint_vref_mv;
> > >
> > > struct ad_sigma_delta sd;
> > > };
> > > @@ -104,17 +117,65 @@ static int ad7780_read_raw(struct iio_dev
> > > *indio_dev,
> > > voltage_uv = regulator_get_voltage(st->reg);
> > > if (voltage_uv < 0)
> > > return voltage_uv;
> > > - *val = (voltage_uv / 1000) * st->gain;
> > > + voltage_uv /= 1000;
> > > + *val = voltage_uv * st->gain;
> > > *val2 = chan->scan_type.realbits - 1;
> > > + st->int_vref_mv = voltage_uv;
> > > return IIO_VAL_FRACTIONAL_LOG2;
> > > case IIO_CHAN_INFO_OFFSET:
> > > *val = -(1 << (chan->scan_type.realbits - 1));
> > > return IIO_VAL_INT;
> > > + case IIO_CHAN_INFO_SAMP_FREQ:
> > > + *val = st->filter;
> > > + return IIO_VAL_INT;
> > > }
> > >
> > > return -EINVAL;
> > > }
> > >
> > > +static int ad7780_write_raw(struct iio_dev *indio_dev,
> > > + struct iio_chan_spec const *chan,
> > > + int val,
> > > + int val2,
> > > + long m)
> > > +{
> > > + struct ad7780_state *st = iio_priv(indio_dev);
> > > + const struct ad7780_chip_info *chip_info = st->chip_info;
> > > + int vref, gain;
> > > + unsigned int full_scale;
> > > +
> > > + if (!chip_info->is_ad778x)
> > > + return 0;
> > > +
> > > + switch (m) {
> > > + case IIO_CHAN_INFO_SCALE:
> > > + if (val != 0)
> > > + return -EINVAL;
> > > +
> > > + vref = st->int_vref_mv * 100LL;
> > From the datasheet, the VREF is ±5 V, therefore your vref variable will
> > overflow. You probably need unsigned long long.
> You're right, I'll fix this in v3.
> >
> >
> > >
> > > + full_scale = 1 << (chip_info-
> > > >channel.scane_type.realbis
> > > - 1);
> > > + gain = DIV_ROUND_CLOSEST(vref, full_scale);
> > > + gain =
Re: [PATCH v2 1/2] staging: iio: ad7780: add gain & filter gpio support
On 01/30, Popa, Stefan Serban wrote:
On Du, 2019-01-27 at 18:30 -0200, Renato Lui Geh wrote:
Previously, the AD7780 driver only supported gpio for the 'powerdown'
pin. This commit adds suppport for the 'gain' and 'filter' pin.
Signed-off-by: Renato Lui Geh
Signed-off-by: Giuliano Belinassi
Co-developed-by: Giuliano Belinassi
---
Changes in v2:
- Filter reading changed to mHz
- Storing filter, gain and voltage to chip_info
Hi,
Hi Stefan,
Thanks for the review. Comments inline.
Renato
Comments inline.
Regards,
Stefan
drivers/staging/iio/adc/ad7780.c | 103 ++---
include/linux/iio/adc/ad_sigma_delta.h | 5 ++
2 files changed, 99 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7780.c
b/drivers/staging/iio/adc/ad7780.c
index c4a85789c2db..82394e31b168 100644
--- a/drivers/staging/iio/adc/ad7780.c
+++ b/drivers/staging/iio/adc/ad7780.c
@@ -39,6 +39,15 @@
#define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2)
#define AD7170_PATTERN_MASK(AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2)
+#define AD7780_GAIN_GPIO 0
+#define AD7780_FILTER_GPIO 1
+
+#define AD7780_GAIN_MIDPOINT 64
+#define AD7780_FILTER_MIDPOINT 13350
+
+static const unsigned int ad778x_gain[2]= { 1, 128 };
+static const unsigned int ad778x_filter[2] = { 1, 16700 };
I would name this array ad778x_odr_avail or something like that.
Sure
We should also consider adding the option to read the available
sampling frequencies from user space, but let's leave this for another
commit.
Do you mean returning 10 and 16.7 Hz?
Should this be done before sending the driver to the main tree, or is it
ok to do something like that after it has left staging?
+
struct ad7780_chip_info {
struct iio_chan_specchannel;
unsigned intpattern_mask;
@@ -50,7 +59,11 @@ struct ad7780_state {
const struct ad7780_chip_info *chip_info;
struct regulator*reg;
struct gpio_desc*powerdown_gpio;
- unsigned intgain;
+ struct gpio_desc*gain_gpio;
+ struct gpio_desc*filter_gpio;
+ unsigned intgain;
+ unsigned intfilter;
Also, this could be renamed as odr.
+ unsigned intint_vref_mv;
struct ad_sigma_delta sd;
};
@@ -104,17 +117,65 @@ static int ad7780_read_raw(struct iio_dev
*indio_dev,
voltage_uv = regulator_get_voltage(st->reg);
if (voltage_uv < 0)
return voltage_uv;
- *val = (voltage_uv / 1000) * st->gain;
+ voltage_uv /= 1000;
+ *val = voltage_uv * st->gain;
*val2 = chan->scan_type.realbits - 1;
+ st->int_vref_mv = voltage_uv;
return IIO_VAL_FRACTIONAL_LOG2;
case IIO_CHAN_INFO_OFFSET:
*val = -(1 << (chan->scan_type.realbits - 1));
return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ *val = st->filter;
+ return IIO_VAL_INT;
}
return -EINVAL;
}
+static int ad7780_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long m)
+{
+ struct ad7780_state *st = iio_priv(indio_dev);
+ const struct ad7780_chip_info *chip_info = st->chip_info;
+ int vref, gain;
+ unsigned int full_scale;
+
+ if (!chip_info->is_ad778x)
+ return 0;
+
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ if (val != 0)
+ return -EINVAL;
+
+ vref = st->int_vref_mv * 100LL;
From the datasheet, the VREF is ±5 V, therefore your vref variable will
overflow. You probably need unsigned long long.
You're right, I'll fix this in v3.
+ full_scale = 1 << (chip_info->channel.scane_type.realbis
- 1);
+ gain = DIV_ROUND_CLOSEST(vref, full_scale);
+ gain = DIV_ROUND_CLOSEST(gain, val2);
+ st->gain = gain;
+ if (gain < AD7780_GAIN_MIDPOINT)
+ gain = 0;
+ else
+ gain = 1;
+ gpiod_set_value(st->gain_gpio, gain);
+ break;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT)
+ val = 0;
+ else
+ val = 1;
+ st->filter = ad778x_filter[val];
+ gpiod_set_value(st->filter_gpio, val);
+ break;
+ }
+
+ return 0;
+}
+
static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta,
unsigned int raw_sample)
{
@@ -126,10 +187,8 @@ static int ad7780_postprocess_sample(struct
[PATCH V2 00/10] X86/KVM/Hyper-V: Add HV ept tlb range list flush support in KVM
From: Lan Tianyu This patchset is to introduce hv ept tlb range list flush function support in the KVM MMU component. Flushing ept tlbs of several address range can be done via single hypercall and new list flush function is used in the kvm_mmu_commit_zap_page() and FNAME(sync_page). This patchset also adds more hv ept tlb range flush support in more KVM MMU function. Change since v1: 1) Make flush list as a hlist instead of list in order to keep struct kvm_mmu_page size. 2) Add last_level flag in the struct kvm_mmu_page instead of spte pointer 3) Move tlb flush from kvm_mmu_notifier_clear_flush_young() to kvm_age_hva() 4) Use range flush in the kvm_vm_ioctl_get/clear_dirty_log() Lan Tianyu (10): X86/Hyper-V: Add parameter offset for hyperv_fill_flush_guest_mapping_list() KVM/VMX: Fill range list in kvm_fill_hv_flush_list_func() KVM/MMU: Add last_level in the struct mmu_spte_page KVM/MMU: Introduce tlb flush with range list KVM/MMU: Flush tlb with range list in sync_page() KVM/MMU: Flush tlb directly in the kvm_mmu_slot_gfn_write_protect() KVM: Add kvm_get_memslot() to get memslot via slot id KVM: Use tlb range flush in the kvm_vm_ioctl_get/clear_dirty_log() KVM: Add flush parameter for kvm_age_hva() KVM/MMU: Use tlb range flush in the kvm_age_hva() arch/arm/include/asm/kvm_host.h | 3 ++- arch/arm64/include/asm/kvm_host.h | 3 ++- arch/mips/include/asm/kvm_host.h| 3 ++- arch/mips/kvm/mmu.c | 11 ++-- arch/powerpc/include/asm/kvm_host.h | 3 ++- arch/powerpc/kvm/book3s.c | 10 ++-- arch/powerpc/kvm/e500_mmu_host.c| 3 ++- arch/x86/hyperv/nested.c| 4 +-- arch/x86/include/asm/kvm_host.h | 11 +++- arch/x86/include/asm/mshyperv.h | 2 +- arch/x86/kvm/mmu.c | 51 + arch/x86/kvm/mmu.h | 7 + arch/x86/kvm/paging_tmpl.h | 15 --- arch/x86/kvm/vmx/vmx.c | 18 +++-- arch/x86/kvm/x86.c | 16 +--- include/linux/kvm_host.h| 1 + virt/kvm/arm/mmu.c | 13 -- virt/kvm/kvm_main.c | 51 +++-- 18 files changed, 160 insertions(+), 65 deletions(-) -- 2.14.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions
On Thu, Jan 31, 2019 at 12:47:07PM -0500, Kimberly Brown wrote: On Thu, Jan 31, 2019 at 04:45:35PM +, Michael Kelley wrote: From: Sasha Levin Sent: Thursday, January 31, 2019 7:20 AM > > I've queued this one for hyper-fixes, thanks all! > Actually, please hold off on queuing this one. In a conversation I had yesterday with Kim, they had identified a deadlock. Kim was going to be looking at some revisions to avoid the deadlock. Kim -- please confirm. Michael This is correct. I need to send a v2 of this patch which addresses two issues: Now dropped, thanks for the heads-up. -- Thanks, Sasha ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier
On Fri, Feb 01, 2019 at 11:17:07AM +0100, Stefan Roese wrote: > @@ -1,3 +1,4 @@ > +// SPDX-License-Identifier: GPL-2.0 > /* > * spi-mt7621.c -- MediaTek MT7621 SPI controller driver > * Please convert the entire comment into a C++ comment so it looks more intentional. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: erofs: keep corrupted fs from crashing kernel in erofs_namei()
As Al pointed out, "
... and while we are at it, what happens to
unsigned int nameoff = le16_to_cpu(de[mid].nameoff);
unsigned int matched = min(startprfx, endprfx);
struct qstr dname = QSTR_INIT(data + nameoff,
unlikely(mid >= ndirents - 1) ?
maxsize - nameoff :
le16_to_cpu(de[mid + 1].nameoff) - nameoff);
/* string comparison without already matched prefix */
int ret = dirnamecmp(name, , );
if le16_to_cpu(de[...].nameoff) is not monotonically increasing? I.e.
what's to prevent e.g. (unsigned)-1 ending up in dname.len?
Corrupted fs image shouldn't oops the kernel.. "
Revisit the related lookup flow to address the issue.
Fixes: d72d1ce60174 ("staging: erofs: add namei functions")
Cc: # 4.19+
Suggested-by: Al Viro
Signed-off-by: Gao Xiang
---
[ It should be better get reviewed first and for linux-next... ]
change log v2:
- fix the missing "kunmap_atomic" pointed out by Dan;
- minor cleanup;
drivers/staging/erofs/namei.c | 187 ++
1 file changed, 99 insertions(+), 88 deletions(-)
diff --git a/drivers/staging/erofs/namei.c b/drivers/staging/erofs/namei.c
index 5596c52e246d..321c881d720f 100644
--- a/drivers/staging/erofs/namei.c
+++ b/drivers/staging/erofs/namei.c
@@ -15,74 +15,77 @@
#include
-/* based on the value of qn->len is accurate */
-static inline int dirnamecmp(struct qstr *qn,
- struct qstr *qd, unsigned int *matched)
+struct erofs_qstr {
+ const unsigned char *name;
+ const unsigned char *end;
+};
+
+/* based on the end of qn is accurate and it must have the trailing '\0' */
+static inline int dirnamecmp(const struct erofs_qstr *qn,
+const struct erofs_qstr *qd,
+unsigned int *matched)
{
- unsigned int i = *matched, len = min(qn->len, qd->len);
-loop:
- if (unlikely(i >= len)) {
- *matched = i;
- if (qn->len < qd->len) {
- /*
-* actually (qn->len == qd->len)
-* when qd->name[i] == '\0'
-*/
- return qd->name[i] == '\0' ? 0 : -1;
+ unsigned int i = *matched;
+
+ /*
+* on-disk error, let's only BUG_ON in the debugging mode.
+* otherwise, it will return 1 to just skip the invalid name
+* and go on (in consideration of the lookup performance).
+*/
+ DBG_BUGON(qd->name > qd->end);
+
+ /* qd could not have trailing '\0' */
+ /* However it is absolutely safe if < qd->end */
+ while (qd->name + i < qd->end && qd->name[i] != '\0') {
+ if (qn->name[i] != qd->name[i]) {
+ *matched = i;
+ return qn->name[i] > qd->name[i] ? 1 : -1;
}
- return (qn->len > qd->len);
+ ++i;
}
-
- if (qn->name[i] != qd->name[i]) {
- *matched = i;
- return qn->name[i] > qd->name[i] ? 1 : -1;
- }
-
- ++i;
- goto loop;
+ *matched = i;
+ /* See comments in __d_alloc on the terminating NUL character */
+ return qn->name[i] == '\0' ? 0 : 1;
}
-static struct erofs_dirent *find_target_dirent(
- struct qstr *name,
- u8 *data, int maxsize)
+#define nameoff_from_disk(off, sz) (le16_to_cpu(off) & ((sz) - 1))
+
+static struct erofs_dirent *find_target_dirent(struct erofs_qstr *name,
+ u8 *data,
+ unsigned int dirblksize,
+ const int ndirents)
{
- unsigned int ndirents, head, back;
+ int head, back;
unsigned int startprfx, endprfx;
struct erofs_dirent *const de = (struct erofs_dirent *)data;
- /* make sure that maxsize is valid */
- BUG_ON(maxsize < sizeof(struct erofs_dirent));
-
- ndirents = le16_to_cpu(de->nameoff) / sizeof(*de);
-
- /* corrupted dir (may be unnecessary...) */
- BUG_ON(!ndirents);
-
- head = 0;
+ /* since the 1st dirent has been evaluated previously */
+ head = 1;
back = ndirents - 1;
startprfx = endprfx = 0;
while (head <= back) {
- unsigned int mid = head + (back - head) / 2;
- unsigned int nameoff = le16_to_cpu(de[mid].nameoff);
+ const int mid = head + (back - head) / 2;
+ const int nameoff = nameoff_from_disk(de[mid].nameoff,
+ dirblksize);
unsigned int matched = min(startprfx, endprfx);
-
- struct qstr dname = QSTR_INIT(data + nameoff,
- unlikely(mid >= ndirents - 1) ?
- maxsize - nameoff :
- le16_to_cpu(de[mid +
Re: [PATCH 3/3] Staging: media: ipu3: fixed max charecter style issue
Hi Prashanta,
On Sun, Jan 27, 2019 at 10:24:16PM +0530, Prashantha SP wrote:
> fixed coding style issue.
Initial capital letter is preferred.
>
> Signed-off-by: Prashantha SP
> ---
> drivers/staging/media/ipu3/ipu3-css.c | 178 ++
> 1 file changed, 94 insertions(+), 84 deletions(-)
>
> diff --git a/drivers/staging/media/ipu3/ipu3-css.c
> b/drivers/staging/media/ipu3/ipu3-css.c
> index 44c55639389a..466a1a8cc422 100644
> --- a/drivers/staging/media/ipu3/ipu3-css.c
> +++ b/drivers/staging/media/ipu3/ipu3-css.c
> @@ -186,7 +186,8 @@ static bool ipu3_css_queue_enabled(struct ipu3_css_queue
> *q)
> /*** css hw ***/
>
> /* In the style of writesl() defined in include/asm-generic/io.h */
> -static inline void writes(const void *mem, ssize_t count, void __iomem *addr)
> +static inline void writes(const void *mem, ssize_t count,
> + void __iomem *addr)
No need to wrap, it's less than 80 characters.
> {
> if (count >= 4) {
> const u32 *buf = mem;
> @@ -671,8 +672,9 @@ static void ipu3_css_pipeline_cleanup(struct ipu3_css
> *css, unsigned int pipe)
> ipu3_css_pool_cleanup(imgu, >pipes[pipe].pool.obgrid);
>
> for (i = 0; i < IMGU_ABI_NUM_MEMORIES; i++)
> - ipu3_css_pool_cleanup(imgu,
> -
> >pipes[pipe].pool.binary_params_p[i]);
> + ipu3_css_pool_cleanup
> + (imgu,
> + >pipes[pipe].pool.binary_params_p[i]);
> }
>
> /*
> @@ -732,43 +734,44 @@ static int ipu3_css_pipeline_init(struct ipu3_css *css,
> unsigned int pipe)
> goto bad_firmware;
>
> cfg_iter->input_info.res.width =
> -
> css_pipe->queue[IPU3_CSS_QUEUE_IN].fmt.mpix.width;
> + css_pipe->queue[IPU3_CSS_QUEUE_IN].fmt.mpix.width;
Could you pull all similar lines one tab stop right of the line beginning
of the statement? The second lines of the statements are currently aligned
but there are just too many tabs...
> cfg_iter->input_info.res.height =
> -
> css_pipe->queue[IPU3_CSS_QUEUE_IN].fmt.mpix.height;
> + css_pipe->queue[IPU3_CSS_QUEUE_IN].fmt.mpix.height;
> cfg_iter->input_info.padded_width =
> css_pipe->queue[IPU3_CSS_QUEUE_IN].width_pad;
> cfg_iter->input_info.format =
> -
> css_pipe->queue[IPU3_CSS_QUEUE_IN].css_fmt->frame_format;
> + css_pipe->queue[IPU3_CSS_QUEUE_IN].css_fmt->frame_format;
> cfg_iter->input_info.raw_bit_depth =
> css_pipe->queue[IPU3_CSS_QUEUE_IN].css_fmt->bit_depth;
> cfg_iter->input_info.raw_bayer_order =
> css_pipe->queue[IPU3_CSS_QUEUE_IN].css_fmt->bayer_order;
> cfg_iter->input_info.raw_type = IMGU_ABI_RAW_TYPE_BAYER;
>
> - cfg_iter->internal_info.res.width =
> css_pipe->rect[IPU3_CSS_RECT_BDS].width;
> + cfg_iter->internal_info.res.width =
> + css_pipe->rect[IPU3_CSS_RECT_BDS].width;
> cfg_iter->internal_info.res.height =
> -
> css_pipe->rect[IPU3_CSS_RECT_BDS].height;
> + css_pipe->rect[IPU3_CSS_RECT_BDS].height;
> cfg_iter->internal_info.padded_width = bds_width_pad;
> cfg_iter->internal_info.format =
> -
> css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->frame_format;
> + css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->frame_format;
> cfg_iter->internal_info.raw_bit_depth =
> css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->bit_depth;
> cfg_iter->internal_info.raw_bayer_order =
> -
> css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->bayer_order;
> + css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->bayer_order;
> cfg_iter->internal_info.raw_type = IMGU_ABI_RAW_TYPE_BAYER;
>
> cfg_iter->output_info.res.width =
> -
> css_pipe->queue[IPU3_CSS_QUEUE_OUT].fmt.mpix.width;
> + css_pipe->queue[IPU3_CSS_QUEUE_OUT].fmt.mpix.width;
> cfg_iter->output_info.res.height =
> -
> css_pipe->queue[IPU3_CSS_QUEUE_OUT].fmt.mpix.height;
> + css_pipe->queue[IPU3_CSS_QUEUE_OUT].fmt.mpix.height;
> cfg_iter->output_info.padded_width =
> css_pipe->queue[IPU3_CSS_QUEUE_OUT].width_pad;
> cfg_iter->output_info.format =
> -
> css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->frame_format;
> + css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->frame_format;
> cfg_iter->output_info.raw_bit_depth =
> - css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->bit_depth;
> + css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->bit_depth;
> cfg_iter->output_info.raw_bayer_order =
> -
[PATCH 8/9 v3] staging: spi: mt7621: Use macros instead of hardcoded values
This patch uses macros instead of hardcoded values for the SPI_MASTER
register access in mt7621_spi_reset() and mt7621_spi_prepare().
Signed-off-by: Stefan Roese
Cc: Mark Brown
Cc: Greg Kroah-Hartman
Cc: NeilBrown
Cc: Sankalp Negi
Cc: Chuanhong Guo
Cc: John Crispin
---
v3:
- New patch, changes spilt into separate patches
drivers/staging/mt7621-spi/spi-mt7621.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c
b/drivers/staging/mt7621-spi/spi-mt7621.c
index 89586a895320..d6385220b796 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -37,6 +37,12 @@
#define SPI_CTL_START BIT(8)
#define MT7621_SPI_MASTER 0x28
+#define MASTER_MORE_BUFMODEBIT(2)
+#define MASTER_FULL_DUPLEX BIT(10)
+#define MASTER_RS_CLK_SEL GENMASK(27, 16)
+#define MASTER_RS_CLK_SEL_SHIFT16
+#define MASTER_RS_SLAVE_SELGENMASK(31, 29)
+
#define MT7621_SPI_MOREBUF 0x2c
#define MT7621_SPI_POLAR 0x38
#define MT7621_SPI_SPACE 0x3c
@@ -77,9 +83,13 @@ static void mt7621_spi_reset(struct mt7621_spi *rs)
{
u32 master = mt7621_spi_read(rs, MT7621_SPI_MASTER);
- master |= 7 << 29;
- master |= 1 << 2;
- master &= ~(1 << 10);
+ /*
+* Select SPI device 7, enable "more buffer mode" and disable
+* full-duplex (only half-duplex really works on this chip
+* reliably)
+*/
+ master |= MASTER_RS_SLAVE_SEL | MASTER_MORE_BUFMODE;
+ master &= ~MASTER_FULL_DUPLEX;
mt7621_spi_write(rs, MT7621_SPI_MASTER, master);
rs->pending_write = 0;
@@ -115,8 +125,8 @@ static int mt7621_spi_prepare(struct spi_device *spi,
unsigned int speed)
rate = 2;
reg = mt7621_spi_read(rs, MT7621_SPI_MASTER);
- reg &= ~(0xfff << 16);
- reg |= (rate - 2) << 16;
+ reg &= ~MASTER_RS_CLK_SEL;
+ reg |= (rate - 2) << MASTER_RS_CLK_SEL_SHIFT;
rs->speed = speed;
reg &= ~MT7621_LSB_FIRST;
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 9/9 v3] staging: spi: mt7621: Remove superfluous pre-declaration of struct mt7621_spi
This patch removes the superfluous pre-declaration of struct mt7621_spi.
Signed-off-by: Stefan Roese
Cc: Mark Brown
Cc: Greg Kroah-Hartman
Cc: NeilBrown
Cc: Sankalp Negi
Cc: Chuanhong Guo
Cc: John Crispin
---
v3:
- New patch, changes spilt into separate patches
drivers/staging/mt7621-spi/spi-mt7621.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c
b/drivers/staging/mt7621-spi/spi-mt7621.c
index d6385220b796..167d0f09823b 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -51,8 +51,6 @@
#define MT7621_CPOLBIT(4)
#define MT7621_LSB_FIRST BIT(3)
-struct mt7621_spi;
-
struct mt7621_spi {
struct spi_master *master;
void __iomem*base;
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/9 v3] staging: spi: mt7621: Use recommended comment style
This patch changes some comments to use the recommended multi-line
comment style.
Signed-off-by: Stefan Roese
Cc: Mark Brown
Cc: Greg Kroah-Hartman
Cc: NeilBrown
Cc: Sankalp Negi
Cc: Chuanhong Guo
Cc: John Crispin
---
v3:
- New patch, changes spilt into separate patches
drivers/staging/mt7621-spi/spi-mt7621.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c
b/drivers/staging/mt7621-spi/spi-mt7621.c
index e548f816ea9e..00cff75ee7ac 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -123,10 +123,10 @@ static int mt7621_spi_prepare(struct spi_device *spi,
unsigned int speed)
if (spi->mode & SPI_LSB_FIRST)
reg |= MT7621_LSB_FIRST;
- /* This SPI controller seems to be tested on SPI flash only
-* and some bits are swizzled under other SPI modes probably
-* due to incorrect wiring inside the silicon. Only mode 0
-* works correctly.
+ /*
+* This SPI controller seems to be tested on SPI flash only and some
+* bits are swizzled under other SPI modes probably due to incorrect
+* wiring inside the silicon. Only mode 0 works correctly.
*/
reg &= ~(MT7621_CPHA | MT7621_CPOL);
@@ -155,9 +155,10 @@ static inline int mt7621_spi_wait_till_ready(struct
mt7621_spi *rs)
static void mt7621_spi_read_half_duplex(struct mt7621_spi *rs,
int rx_len, u8 *buf)
{
- /* Combine with any pending write, and perform one or
-* more half-duplex transactions reading 'len' bytes.
-* Data to be written is already in MT7621_SPI_DATA*
+ /*
+* Combine with any pending write, and perform one or more half-duplex
+* transactions reading 'len' bytes. Data to be written is already in
+* MT7621_SPI_DATA.
*/
int tx_len = rs->pending_write;
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier
Adopt the SPDX license identifier headers to ease license compliance management. Signed-off-by: Stefan Roese Cc: Mark Brown Cc: Greg Kroah-Hartman Cc: NeilBrown Cc: Sankalp Negi Cc: Chuanhong Guo Cc: John Crispin --- v3: - No change v2: - Changes are done to the driver in staging before moving it out of staging into drivers/spi drivers/staging/mt7621-spi/spi-mt7621.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c index 513b6e79b985..c2f6f9ce52a2 100644 --- a/drivers/staging/mt7621-spi/spi-mt7621.c +++ b/drivers/staging/mt7621-spi/spi-mt7621.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * spi-mt7621.c -- MediaTek MT7621 SPI controller driver * @@ -8,10 +9,6 @@ * Some parts are based on spi-orion.c: * Author: Shadi Ammouri * Copyright (C) 2007-2008 Marvell Ltd. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #include -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/9 v3] staging: spi: mt7621: Remove superfluous SPI_BPW_MASK definition
This patch removes the SPI_BPW_MASK definition as this is already available in include/linux/spi/spi.h. Signed-off-by: Stefan Roese Cc: Mark Brown Cc: Greg Kroah-Hartman Cc: NeilBrown Cc: Sankalp Negi Cc: Chuanhong Guo Cc: John Crispin --- v3: - New patch, changes spilt into separate patches drivers/staging/mt7621-spi/spi-mt7621.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c index ca4632740827..fe9c863af1cc 100644 --- a/drivers/staging/mt7621-spi/spi-mt7621.c +++ b/drivers/staging/mt7621-spi/spi-mt7621.c @@ -19,8 +19,6 @@ #include #include -#define SPI_BPW_MASK(bits) BIT((bits) - 1) - #define DRIVER_NAME"spi-mt7621" /* in usec */ #define RALINK_SPI_WAIT_MAX_LOOP 2000 -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/9 v3] staging: spi: mt7621: Sort register definitions
This patch sorts the SPI register definitions by increasing addresses. Signed-off-by: Stefan Roese Cc: Mark Brown Cc: Greg Kroah-Hartman Cc: NeilBrown Cc: Sankalp Negi Cc: Chuanhong Guo Cc: John Crispin --- v3: - New patch, changes spilt into separate patches drivers/staging/mt7621-spi/spi-mt7621.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c index 00cff75ee7ac..89586a895320 100644 --- a/drivers/staging/mt7621-spi/spi-mt7621.c +++ b/drivers/staging/mt7621-spi/spi-mt7621.c @@ -36,9 +36,9 @@ #define SPI_CTL_TX_RX_CNT_MASK 0xff #define SPI_CTL_START BIT(8) -#define MT7621_SPI_POLAR 0x38 #define MT7621_SPI_MASTER 0x28 #define MT7621_SPI_MOREBUF 0x2c +#define MT7621_SPI_POLAR 0x38 #define MT7621_SPI_SPACE 0x3c #define MT7621_CPHABIT(5) -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/9 v3] staging: spi: mt7621: Clean up excessive header usage
This patch removes unnecessary header includes and sorts the remaining ones alphabetically. Signed-off-by: Stefan Roese Cc: Mark Brown Cc: Greg Kroah-Hartman Cc: NeilBrown Cc: Sankalp Negi Cc: Chuanhong Guo Cc: John Crispin --- v3: - New patch, changes spilt into separate patches drivers/staging/mt7621-spi/spi-mt7621.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c index c2f6f9ce52a2..440c3f77fde8 100644 --- a/drivers/staging/mt7621-spi/spi-mt7621.c +++ b/drivers/staging/mt7621-spi/spi-mt7621.c @@ -11,19 +11,13 @@ * Copyright (C) 2007-2008 Marvell Ltd. */ -#include -#include #include -#include #include #include +#include +#include #include #include -#include -#include -#include - -#include #define SPI_BPW_MASK(bits) BIT((bits) - 1) -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/9 v3] staging: spi: mt7621: Add return code check on device_reset()
This patch adds a return code check on device_reset() and removes the
compile warning.
Signed-off-by: Stefan Roese
Cc: Mark Brown
Cc: Greg Kroah-Hartman
Cc: NeilBrown
Cc: Sankalp Negi
Cc: Chuanhong Guo
Cc: John Crispin
---
v3:
- New patch, changes spilt into separate patches
drivers/staging/mt7621-spi/spi-mt7621.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c
b/drivers/staging/mt7621-spi/spi-mt7621.c
index 440c3f77fde8..ca4632740827 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -321,6 +321,7 @@ static int mt7621_spi_probe(struct platform_device *pdev)
int status = 0;
struct clk *clk;
struct mt7621_spi_ops *ops;
+ int ret;
match = of_match_device(mt7621_spi_match, >dev);
if (!match)
@@ -368,7 +369,11 @@ static int mt7621_spi_probe(struct platform_device *pdev)
rs->pending_write = 0;
dev_info(>dev, "sys_freq: %u\n", rs->sys_freq);
- device_reset(>dev);
+ ret = device_reset(>dev);
+ if (ret) {
+ dev_err(>dev, "SPI reset failed!\n");
+ return ret;
+ }
mt7621_spi_reset(rs);
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/9 v3] staging: spi: mt7621: Minor cosmetic changes
Align macro definitions and add some empty lines to make the code better readable. Signed-off-by: Stefan Roese Cc: Mark Brown Cc: Greg Kroah-Hartman Cc: NeilBrown Cc: Sankalp Negi Cc: Chuanhong Guo Cc: John Crispin --- v3: - New patch, changes spilt into separate patches drivers/staging/mt7621-spi/spi-mt7621.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c index fe9c863af1cc..e548f816ea9e 100644 --- a/drivers/staging/mt7621-spi/spi-mt7621.c +++ b/drivers/staging/mt7621-spi/spi-mt7621.c @@ -19,12 +19,13 @@ #include #include -#define DRIVER_NAME"spi-mt7621" +#define DRIVER_NAME"spi-mt7621" + /* in usec */ -#define RALINK_SPI_WAIT_MAX_LOOP 2000 +#define RALINK_SPI_WAIT_MAX_LOOP 2000 /* SPISTAT register bit field */ -#define SPISTAT_BUSY BIT(0) +#define SPISTAT_BUSY BIT(0) #define MT7621_SPI_TRANS 0x00 #define SPITRANS_BUSY BIT(16) @@ -186,6 +187,7 @@ static void mt7621_spi_read_half_duplex(struct mt7621_spi *rs, *buf++ = val & 0xff; val >>= 8; } + rx_len -= i; } } @@ -279,6 +281,7 @@ static int mt7621_spi_transfer_one_message(struct spi_master *master, mt7621_spi_flush(rs); mt7621_spi_set_cs(spi, 0); + msg_done: m->status = status; spi_finalize_current_message(master); -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2 v2] staging: spi: mt7621: Minor code cleanup
On Fri, Feb 01 2019, Stefan Roese wrote: > On 01.02.19 10:03, Greg Kroah-Hartman wrote: >> On Fri, Feb 01, 2019 at 09:57:12AM +0100, Stefan Roese wrote: >>> This patch cleans up some minor issues with this driver: >>> - Remove unnecessary header includes >>> - Sort header alphabetically >>> - Use correct comment style >>> - Add return code check on device_reset() >>> - Remove SPI_BPW_MASK definition (already available in >>>include/linux/spi/spi.h) >>> - Use macros instead of hardcoded values for SPI_MASTER register access >>>as suggested by Neil Brown (in mt7621_spi_reset and mt7621_spi_prepare) >> >> When you have to start listing the different things you do in a patch, >> that's a huge sign you need to break this up into different patches :) > > I personally find this over complex for these type of changes, that's > why I prefer to send such "minor" changes in a single patch. But I > can definitely split this up into multiple patches, if this is what's > preferred by you (and others). I'll put my hand as an "other". I understand the temptation to combine changes and I've been guilty of it occasionally myself, but it is generally best avoided. The key issue for me is ease of review. I like to be able to look at the patch description, understand it, then look at the code changes and think "yes, that looks right" though with more complex changes I'll need to study the code are bit more closely of course. I would probably keep >>> - Remove unnecessary header includes >>> - Sort header alphabetically together, but each of the other changes should be one-per-patch. You also: - made some white-space changes - deleted a point pre-declaration of struct mt7621_spi The white-space changes could be combined with comment-style fixes, the deletion could be combined with the SPI_BPW_MASK deletation. But in any case, they should be mentioned. Thanks - these are all valuable improvements. NeilBrown > >> Please do that here, it should be a series, each one doing a single type >> of thing. > > Sure, will do. > > Thanks, > Stefan signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2 v2] staging: spi: mt7621: Minor code cleanup
On Fri, Feb 01, 2019 at 10:16:23AM +0100, Stefan Roese wrote: > On 01.02.19 10:03, Greg Kroah-Hartman wrote: > > On Fri, Feb 01, 2019 at 09:57:12AM +0100, Stefan Roese wrote: > > > This patch cleans up some minor issues with this driver: > > > - Remove unnecessary header includes > > > - Sort header alphabetically > > > - Use correct comment style > > > - Add return code check on device_reset() > > > - Remove SPI_BPW_MASK definition (already available in > > >include/linux/spi/spi.h) > > > - Use macros instead of hardcoded values for SPI_MASTER register access > > >as suggested by Neil Brown (in mt7621_spi_reset and mt7621_spi_prepare) > > > > When you have to start listing the different things you do in a patch, > > that's a huge sign you need to break this up into different patches :) > > I personally find this over complex for these type of changes, that's > why I prefer to send such "minor" changes in a single patch. But I > can definitely split this up into multiple patches, if this is what's > preferred by you (and others). When you review as many patches as we do, it's _MUCH_ easier to review a patch that only does one thing, to verify it really does that one thing properly. If you mix it all up, it takes a lot more effort to try to review it all correctly. Remember, you need to make it as easy as possible to understand the change to make me seem horrible to reject your patch :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2 v2] staging: spi: mt7621: Minor code cleanup
On 01.02.19 10:03, Greg Kroah-Hartman wrote: On Fri, Feb 01, 2019 at 09:57:12AM +0100, Stefan Roese wrote: This patch cleans up some minor issues with this driver: - Remove unnecessary header includes - Sort header alphabetically - Use correct comment style - Add return code check on device_reset() - Remove SPI_BPW_MASK definition (already available in include/linux/spi/spi.h) - Use macros instead of hardcoded values for SPI_MASTER register access as suggested by Neil Brown (in mt7621_spi_reset and mt7621_spi_prepare) When you have to start listing the different things you do in a patch, that's a huge sign you need to break this up into different patches :) I personally find this over complex for these type of changes, that's why I prefer to send such "minor" changes in a single patch. But I can definitely split this up into multiple patches, if this is what's preferred by you (and others). Please do that here, it should be a series, each one doing a single type of thing. Sure, will do. Thanks, Stefan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2 v2] staging: spi: mt7621: Minor code cleanup
This patch cleans up some minor issues with this driver:
- Remove unnecessary header includes
- Sort header alphabetically
- Use correct comment style
- Add return code check on device_reset()
- Remove SPI_BPW_MASK definition (already available in
include/linux/spi/spi.h)
- Use macros instead of hardcoded values for SPI_MASTER register access
as suggested by Neil Brown (in mt7621_spi_reset and mt7621_spi_prepare)
Signed-off-by: Stefan Roese
Cc: Mark Brown
Cc: Greg Kroah-Hartman
Cc: NeilBrown
Cc: Sankalp Negi
Cc: Chuanhong Guo
Cc: John Crispin
---
v2:
- Changes are done to the driver in staging before moving it out of
staging into drivers/spi
- Remove SPI_BPW_MASK macro
- Use macros instead of hardcoded values for SPI_MASTER bits
- Remove code cleanup comment from TODO file
drivers/staging/mt7621-spi/TODO | 1 -
drivers/staging/mt7621-spi/spi-mt7621.c | 65 ++---
2 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/mt7621-spi/TODO b/drivers/staging/mt7621-spi/TODO
index fdbc5002c32a..126cc80c7c68 100644
--- a/drivers/staging/mt7621-spi/TODO
+++ b/drivers/staging/mt7621-spi/TODO
@@ -1,5 +1,4 @@
-- general code review and clean up
- ensure device-tree requirements are documented
Cc: NeilBrown
diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c
b/drivers/staging/mt7621-spi/spi-mt7621.c
index c2f6f9ce52a2..167d0f09823b 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -11,28 +11,21 @@
* Copyright (C) 2007-2008 Marvell Ltd.
*/
-#include
-#include
#include
-#include
#include
#include
+#include
+#include
#include
#include
-#include
-#include
-#include
-#include
+#define DRIVER_NAME"spi-mt7621"
-#define SPI_BPW_MASK(bits) BIT((bits) - 1)
-
-#define DRIVER_NAME"spi-mt7621"
/* in usec */
-#define RALINK_SPI_WAIT_MAX_LOOP 2000
+#define RALINK_SPI_WAIT_MAX_LOOP 2000
/* SPISTAT register bit field */
-#define SPISTAT_BUSY BIT(0)
+#define SPISTAT_BUSY BIT(0)
#define MT7621_SPI_TRANS 0x00
#define SPITRANS_BUSY BIT(16)
@@ -43,17 +36,21 @@
#define SPI_CTL_TX_RX_CNT_MASK 0xff
#define SPI_CTL_START BIT(8)
-#define MT7621_SPI_POLAR 0x38
#define MT7621_SPI_MASTER 0x28
+#define MASTER_MORE_BUFMODEBIT(2)
+#define MASTER_FULL_DUPLEX BIT(10)
+#define MASTER_RS_CLK_SEL GENMASK(27, 16)
+#define MASTER_RS_CLK_SEL_SHIFT16
+#define MASTER_RS_SLAVE_SELGENMASK(31, 29)
+
#define MT7621_SPI_MOREBUF 0x2c
+#define MT7621_SPI_POLAR 0x38
#define MT7621_SPI_SPACE 0x3c
#define MT7621_CPHABIT(5)
#define MT7621_CPOLBIT(4)
#define MT7621_LSB_FIRST BIT(3)
-struct mt7621_spi;
-
struct mt7621_spi {
struct spi_master *master;
void __iomem*base;
@@ -84,9 +81,13 @@ static void mt7621_spi_reset(struct mt7621_spi *rs)
{
u32 master = mt7621_spi_read(rs, MT7621_SPI_MASTER);
- master |= 7 << 29;
- master |= 1 << 2;
- master &= ~(1 << 10);
+ /*
+* Select SPI device 7, enable "more buffer mode" and disable
+* full-duplex (only half-duplex really works on this chip
+* reliably)
+*/
+ master |= MASTER_RS_SLAVE_SEL | MASTER_MORE_BUFMODE;
+ master &= ~MASTER_FULL_DUPLEX;
mt7621_spi_write(rs, MT7621_SPI_MASTER, master);
rs->pending_write = 0;
@@ -122,18 +123,18 @@ static int mt7621_spi_prepare(struct spi_device *spi,
unsigned int speed)
rate = 2;
reg = mt7621_spi_read(rs, MT7621_SPI_MASTER);
- reg &= ~(0xfff << 16);
- reg |= (rate - 2) << 16;
+ reg &= ~MASTER_RS_CLK_SEL;
+ reg |= (rate - 2) << MASTER_RS_CLK_SEL_SHIFT;
rs->speed = speed;
reg &= ~MT7621_LSB_FIRST;
if (spi->mode & SPI_LSB_FIRST)
reg |= MT7621_LSB_FIRST;
- /* This SPI controller seems to be tested on SPI flash only
-* and some bits are swizzled under other SPI modes probably
-* due to incorrect wiring inside the silicon. Only mode 0
-* works correctly.
+ /*
+* This SPI controller seems to be tested on SPI flash only and some
+* bits are swizzled under other SPI modes probably due to incorrect
+* wiring inside the silicon. Only mode 0 works correctly.
*/
reg &= ~(MT7621_CPHA | MT7621_CPOL);
@@ -162,9 +163,10 @@ static inline int mt7621_spi_wait_till_ready(struct
mt7621_spi *rs)
static void mt7621_spi_read_half_duplex(struct mt7621_spi *rs,
int rx_len, u8 *buf)
{
- /* Combine with any pending write, and perform one or
-* more half-duplex transactions reading 'len' bytes.
-* Data to be written is already in MT7621_SPI_DATA*
+ /*
+* Combine with any pending write, and perform
[PATCH 1/2 v2] staging: spi: mt7621: Switch to SPDX identifier
Adopt the SPDX license identifier headers to ease license compliance management. Signed-off-by: Stefan Roese Cc: Mark Brown Cc: Greg Kroah-Hartman Cc: NeilBrown Cc: Sankalp Negi Cc: Chuanhong Guo Cc: John Crispin --- v2: - Changes are done to the driver in staging before moving it out of staging into drivers/spi drivers/staging/mt7621-spi/spi-mt7621.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c index 513b6e79b985..c2f6f9ce52a2 100644 --- a/drivers/staging/mt7621-spi/spi-mt7621.c +++ b/drivers/staging/mt7621-spi/spi-mt7621.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * spi-mt7621.c -- MediaTek MT7621 SPI controller driver * @@ -8,10 +9,6 @@ * Some parts are based on spi-orion.c: * Author: Shadi Ammouri * Copyright (C) 2007-2008 Marvell Ltd. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #include -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2 v2] staging: spi: mt7621: Minor code cleanup
On Fri, Feb 01, 2019 at 09:57:12AM +0100, Stefan Roese wrote: > This patch cleans up some minor issues with this driver: > - Remove unnecessary header includes > - Sort header alphabetically > - Use correct comment style > - Add return code check on device_reset() > - Remove SPI_BPW_MASK definition (already available in > include/linux/spi/spi.h) > - Use macros instead of hardcoded values for SPI_MASTER register access > as suggested by Neil Brown (in mt7621_spi_reset and mt7621_spi_prepare) When you have to start listing the different things you do in a patch, that's a huge sign you need to break this up into different patches :) Please do that here, it should be a series, each one doing a single type of thing. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: fix sys heap pool's gfp_flags
On 2019/2/1 16:15, Dan Carpenter wrote:
On Fri, Feb 01, 2019 at 02:59:46PM +0800, Qing Xia wrote:
In the first loop, gfp_flags will be modified to high_order_gfp_flags,
and there will be no chance to change back to low_order_gfp_flags.
Fixes: e7f63771 ("ION: Sys_heap: Add cached pool to spead up cached buffer
alloc")
Huh... Presumably you found this bug just by reading the code. I
wonder how it affects runtime?
The problem is that when I found that there was no page allocation failure
warning
after the failure of the ion alloc in my test, and then I found the problem of
gfp_flags.
regards,
Qing
regards,
dan carpenter
.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: fix sys heap pool's gfp_flags
On Fri, Feb 01, 2019 at 02:59:46PM +0800, Qing Xia wrote:
> In the first loop, gfp_flags will be modified to high_order_gfp_flags,
> and there will be no chance to change back to low_order_gfp_flags.
>
> Fixes: e7f63771 ("ION: Sys_heap: Add cached pool to spead up cached buffer
> alloc")
Huh... Presumably you found this bug just by reading the code. I
wonder how it affects runtime?
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
