* Re: [PATCH 0/4] staging: wilc1000: make use of descriptor-based interface for GPIO
From: Claudiu Beznea @ 2018-07-24 8:04 UTC (permalink / raw)
To: Ajay Singh, linux-wireless
Cc: devel, gregkh, ganesh.krishna, venkateswara.kaja, aditya.shankar,
adham.abozaeid, robh+dt, mark.rutland, linus.walleij
In-Reply-To: <1532088098-8483-1-git-send-email-ajay.kathat@microchip.com>
Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>
On 20.07.2018 15:01, Ajay Singh wrote:
> This patch series contains changes mainly related to make use of
> descriptor-based interface instead of integer-based interface for GPIO.
> Modified the compatible string to use 'microchip' instead of 'atmel' prefix.
> Also added the DT binding reference file.
>
> This patch is created on top of [1] patch series.
>
> [1]. https://www.spinics.net/lists/linux-wireless/msg175360.html
>
> Ajay Singh (4):
> staging: wilc1000: remove gpio parameter from wilc_netdev_init()
> staging: wilc1000: rename variable from 'gpio' to 'gpio_irq'
> staging: wilc1000: change compatible string from atmel to microchip
> staging: wilc1000: use descriptor-based interface for GPIO
>
> drivers/staging/wilc1000/TODO | 4 ---
> drivers/staging/wilc1000/linux_wlan.c | 42 +++++++++-------------
> .../staging/wilc1000/microchip,wilc1000,sdio.txt | 32 +++++++++++++++++
> .../staging/wilc1000/microchip,wilc1000,spi.txt | 26 ++++++++++++++
> drivers/staging/wilc1000/wilc_sdio.c | 38 ++++++++++++++------
> drivers/staging/wilc1000/wilc_spi.c | 40 +++++++++++++--------
> drivers/staging/wilc1000/wilc_wfi_netdevice.h | 5 +--
> 7 files changed, 130 insertions(+), 57 deletions(-)
> create mode 100644 drivers/staging/wilc1000/microchip,wilc1000,sdio.txt
> create mode 100644 drivers/staging/wilc1000/microchip,wilc1000,spi.txt
>
^ permalink raw reply
* Re: [PATCH v2 2/2] app/testpmd: fix help string for tm commit cmd
From: Iremonger, Bernard @ 2018-07-24 9:08 UTC (permalink / raw)
To: Krzysztof Kanas, dev@dpdk.org, Lu, Wenzhuo, Wu, Jingjing
Cc: Nithin Dabilpuram, Singh, Jasvinder
In-Reply-To: <20180718095443.31429-1-krzysztof.kanas@caviumnetworks.com>
Hi Krzysztof
> -----Original Message-----
> From: Krzysztof Kanas [mailto:krzysztof.kanas@caviumnetworks.com]
> Sent: Wednesday, July 18, 2018 10:55 AM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>;
> krzysztof.kanas@caviumnetworks.com; dev@dpdk.org; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: Nithin Dabilpuram <nithin.dabilpuram@cavium.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> Subject: [PATCH v2 2/2] app/testpmd: fix help string for tm commit cmd
>
> Fixes: bd475cefc7cb ("app/testpmd: fix use of uninitialized field")
> Cc: krzysztof.kanas@caviumnetworks.com
>
> Signed-off-by: Krzysztof Kanas <krzysztof.kanas@caviumnetworks.com>
> ---
> v2:
> * Fix the Fixes commit message line
> ---
> app/test-pmd/cmdline_tm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/cmdline_tm.c b/app/test-pmd/cmdline_tm.c index
> 4f515241703a..09acc1b87293 100644
> --- a/app/test-pmd/cmdline_tm.c
> +++ b/app/test-pmd/cmdline_tm.c
> @@ -2172,7 +2172,7 @@ static void
> cmd_port_tm_hierarchy_commit_parsed(void *parsed_result,
> cmdline_parse_inst_t cmd_port_tm_hierarchy_commit = {
> .f = cmd_port_tm_hierarchy_commit_parsed,
> .data = NULL,
> - .help_str = "Set port tm node shaper profile",
> + .help_str = "Commit port tm hierarchy",
> .tokens = {
> (void *)&cmd_port_tm_hierarchy_commit_port,
> (void *)&cmd_port_tm_hierarchy_commit_tm,
> --
> 2.18.0
The check-git-log script is still showing an error:
./devtools/check-git-log.sh -1
Wrong 'Fixes' reference:
Fixes: bd475cefc7cb ("app/testpmd: fix use of uninitialized field")
It is better to send the complete patch set (2 patches) with each revision.
My ack can be carried forward on the first patch.
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH 0/6] Symbol namespaces
From: Arnd Bergmann @ 2018-07-24 9:08 UTC (permalink / raw)
To: Martijn Coenen
Cc: Linux Kernel Mailing List, Masahiro Yamada, Michal Marek,
Geert Uytterhoeven, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
the arch/x86 maintainers, Alan Stern, Greg Kroah-Hartman,
Oliver Neukum, Jessica Yu, Stephen Boyd, Philippe Ombredanne,
Kate Stewart, Sam Ravnborg, Linux Kbuild mailing list, linux-m68k,
USB
In-Reply-To: <CAB0TPYH2G_+faZDcvoEyUmZfwP0j83P0KeZXSJdN9rJ4M1ad3w@mail.gmail.com>
On Tue, Jul 24, 2018 at 10:09 AM, Martijn Coenen <maco@android.com> wrote:
> Hi Arnd,
>
> On Mon, Jul 23, 2018 at 4:28 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> This looks nice. I don't want to overload you with additional
>> requests, but it might be good to think about how this can
>> be taken further, if we want to actually convert large
>> parts of the kernel to it.
>
> No worries about overloading, I'm happy with all feedback to make this better.
>
>> I have two ideas:
>>
>> - It would be nice to have a simple mechanism in Kconfig
>> to put all symbols in a module into a namespace that is
>> derived from KBUILD_MODNAME, to avoid having to
>> annotate every symbol separately. This could be similar
>> to how we ended up dealing with EXPORT_NO_SYMBOLS
>> in the linux-2.4 days, with the goal of eventually having
>> a namespace for each symbol. Similarly, we could pass
>> a number of default namespaces through the Makefile,
>> e.g. if we have a directory that has lots of drivers that all
>> import the symbols from a common subsystem module.
>
> I'm hinging on two thoughts here; I really like it because it
> obviously reduces work and makes this easier to use. On the other
> hand, you now have to look in multiple places to see if a symbol is
> exported to a namespace/imported from a module. Perhaps it depends on
> how coarse-grained the namespaces end up being. Either way, I think it
> would be pretty easy to cook up patches for this.
Ok, maybe try it and see where we get with it?
>> - If this is possible to implement, it would be great to have
>> a way to limit the globally visible symbols in a module
>> to the ones that are explicitly exported even when a that
>> module is built-in rather than loadable. Not sure how this
>> is best done though. A couple of things can be done with
>> objcopy, or possibly by reintroducing partial linking (which
>> was removed not too long ago).
>
> If I understand you correctly, this is orthogonal to symbol
> namespaces, right?
Right, the only connection here is that both try to limit the
scope of which symbols are available where. This can definitely
be done without symbol namespaces. The implementation I
had in mind with objcopy might end up using the same
KBUILD_MODNAME as a prefix for internal symbols (which
are not exported to modules), but that is a different problem.
Another related area is the question what happens to symbols
that we want to share between two built-in parts of the kernel
without exporting them to modules. E.g. if we might want to
put all of vfs into one built-in module and make its symbols
private to that module, while all of the mm subsystem is in a
separate built-in module. The symbols that are needed for
communicating between the two could simply be exported
with EXPORT_SYMBOL_GPL(), but we that would open the
surface to loadable modules that can't access them today.
Using namespaces could solve that problem by limiting the
scope in another way, or we could come up with a different
method to annotate them, such as using the gcc visibility
attributes.
> Do you think these things should be a part of these series, or can it
> be a follow-up?
If you think you can do the first thing easily, I'd say we should
try to come up with an idea of where we want to take this in
the long run. For limiting the scope of global symbols, that may
well be a much bigger task to do upfront, so unless you have an
idea for how to do that, maybe keep it in mind but let's not make
it a dependency.
Arnd
^ permalink raw reply
* Re: [PATCH] mm: thp: remove use_zero_page sysfs knob
From: Kirill A. Shutemov @ 2018-07-24 9:08 UTC (permalink / raw)
To: David Rientjes
Cc: Matthew Wilcox, Yang Shi, Andrew Morton, hughd, aaron.lu,
linux-mm, linux-kernel
In-Reply-To: <alpine.DEB.2.21.1807231427550.103523@chino.kir.corp.google.com>
On Mon, Jul 23, 2018 at 02:33:08PM -0700, David Rientjes wrote:
> On Mon, 23 Jul 2018, David Rientjes wrote:
>
> > > > The huge zero page can be reclaimed under memory pressure and, if it is,
> > > > it is attempted to be allocted again with gfp flags that attempt memory
> > > > compaction that can become expensive. If we are constantly under memory
> > > > pressure, it gets freed and reallocated millions of times always trying to
> > > > compact memory both directly and by kicking kcompactd in the background.
> > > >
> > > > It likely should also be per node.
> > >
> > > Have you benchmarked making the non-huge zero page per-node?
> > >
> >
> > Not since we disable it :) I will, though. The more concerning issue for
> > us, modulo CVE-2017-1000405, is the cpu cost of constantly directly
> > compacting memory for allocating the hzp in real time after it has been
> > reclaimed. We've observed this happening tens or hundreds of thousands
> > of times on some systems. It will be 2MB per node on x86 if the data
> > suggests we should make it NUMA aware, I don't think the cost is too high
> > to leave it persistently available even under memory pressure if
> > use_zero_page is enabled.
> >
>
> Measuring access latency to 4GB of memory on Naples I observe ~6.7%
> slower access latency intrasocket and ~14% slower intersocket.
>
> use_zero_page is currently a simple thp flag, meaning it rejects writes
> where val != !!val, so perhaps it would be best to overload it with
> additional options? I can imagine 0x2 defining persistent allocation so
> that the hzp is not freed when the refcount goes to 0 and 0x4 defining if
> the hzp should be per node. Implementing persistent allocation fixes our
> concern with it, so I'd like to start there. Comments?
Why not a separate files?
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v3 1/1] KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
From: Cédric Le Goater @ 2018-07-24 9:07 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev; +Cc: paulus, kvm-ppc, kvm, david
In-Reply-To: <1fb3aea5f44f1029866ee10db40abde7e18b24ad.1531967105.git.sbobroff@linux.ibm.com>
On 07/19/2018 04:25 AM, Sam Bobroff wrote:
> From: Sam Bobroff <sam.bobroff@au1.ibm.com>
>
> It is not currently possible to create the full number of possible
> VCPUs (KVM_MAX_VCPUS) on Power9 with KVM-HV when the guest uses less
> threads per core than it's core stride (or "VSMT mode"). This is
> because the VCORE ID and XIVE offsets to grow beyond KVM_MAX_VCPUS
> even though the VCPU ID is less than KVM_MAX_VCPU_ID.
>
> To address this, "pack" the VCORE ID and XIVE offsets by using
> knowledge of the way the VCPU IDs will be used when there are less
> guest threads per core than the core stride. The primary thread of
> each core will always be used first. Then, if the guest uses more than
> one thread per core, these secondary threads will sequentially follow
> the primary in each core.
>
> So, the only way an ID above KVM_MAX_VCPUS can be seen, is if the
> VCPUs are being spaced apart, so at least half of each core is empty
> and IDs between KVM_MAX_VCPUS and (KVM_MAX_VCPUS * 2) can be mapped
> into the second half of each core (4..7, in an 8-thread core).
>
> Similarly, if IDs above KVM_MAX_VCPUS * 2 are seen, at least 3/4 of
> each core is being left empty, and we can map down into the second and
> third quarters of each core (2, 3 and 5, 6 in an 8-thread core).
>
> Lastly, if IDs above KVM_MAX_VCPUS * 4 are seen, only the primary
> threads are being used and 7/8 of the core is empty, allowing use of
> the 1, 3, 5 and 7 thread slots.
>
> (Strides less than 8 are handled similarly.)
>
> This allows the VCORE ID or offset to be calculated quickly from the
> VCPU ID or XIVE server numbers, without access to the VCPU structure.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
On the XIVE part,
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> Hello everyone,
>
> I've completed a trial merge with the guest native-XIVE code and found no
> problems; it's no more difficult than the host side and only requires a few
> calls to xive_vp().
>
> On that basis, here is v3 (unchanged from v2) as non-RFC and it seems to be
> ready to go.
>
> Patch set v3:
> Patch 1/1: KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
>
> Patch set v2:
> Patch 1/1: KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
> * Corrected places in kvm/book3s_xive.c where IDs weren't packed.
> * Because kvmppc_pack_vcpu_id() is only called on P9, there is no need to test "emul_smt_mode > 1", so remove it.
> * Re-ordered block_offsets[] to be more ascending.
> * Added more detailed description of the packing algorithm.
>
> Patch set v1:
> Patch 1/1: KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
>
> arch/powerpc/include/asm/kvm_book3s.h | 44 +++++++++++++++++++++++++++++++++++
> arch/powerpc/kvm/book3s_hv.c | 14 +++++++----
> arch/powerpc/kvm/book3s_xive.c | 19 +++++++++------
> 3 files changed, 66 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> index 1f345a0b6ba2..ba4b6e00fca7 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -390,4 +390,48 @@ extern int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu);
> #define SPLIT_HACK_MASK 0xff000000
> #define SPLIT_HACK_OFFS 0xfb000000
>
> +/* Pack a VCPU ID from the [0..KVM_MAX_VCPU_ID) space down to the
> + * [0..KVM_MAX_VCPUS) space, while using knowledge of the guest's core stride
> + * (but not it's actual threading mode, which is not available) to avoid
> + * collisions.
> + *
> + * The implementation leaves VCPU IDs from the range [0..KVM_MAX_VCPUS) (block
> + * 0) unchanged: if the guest is filling each VCORE completely then it will be
> + * using consecutive IDs and it will fill the space without any packing.
> + *
> + * For higher VCPU IDs, the packed ID is based on the VCPU ID modulo
> + * KVM_MAX_VCPUS (effectively masking off the top bits) and then an offset is
> + * added to avoid collisions.
> + *
> + * VCPU IDs in the range [KVM_MAX_VCPUS..(KVM_MAX_VCPUS*2)) (block 1) are only
> + * possible if the guest is leaving at least 1/2 of each VCORE empty, so IDs
> + * can be safely packed into the second half of each VCORE by adding an offset
> + * of (stride / 2).
> + *
> + * Similarly, if VCPU IDs in the range [(KVM_MAX_VCPUS*2)..(KVM_MAX_VCPUS*4))
> + * (blocks 2 and 3) are seen, the guest must be leaving at least 3/4 of each
> + * VCORE empty so packed IDs can be offset by (stride / 4) and (stride * 3 / 4).
> + *
> + * Finally, VCPU IDs from blocks 5..7 will only be seen if the guest is using a
> + * stride of 8 and 1 thread per core so the remaining offsets of 1, 3, 5 and 7
> + * must be free to use.
> + *
> + * (The offsets for each block are stored in block_offsets[], indexed by the
> + * block number if the stride is 8. For cases where the guest's stride is less
> + * than 8, we can re-use the block_offsets array by multiplying the block
> + * number by (MAX_SMT_THREADS / stride) to reach the correct entry.)
> + */
> +static inline u32 kvmppc_pack_vcpu_id(struct kvm *kvm, u32 id)
> +{
> + const int block_offsets[MAX_SMT_THREADS] = {0, 4, 2, 6, 1, 3, 5, 7};
> + int stride = kvm->arch.emul_smt_mode;
> + int block = (id / KVM_MAX_VCPUS) * (MAX_SMT_THREADS / stride);
> + u32 packed_id;
> +
> + BUG_ON(block >= MAX_SMT_THREADS);
> + packed_id = (id % KVM_MAX_VCPUS) + block_offsets[block];
> + BUG_ON(packed_id >= KVM_MAX_VCPUS);
> + return packed_id;
> +}
> +
> #endif /* __ASM_KVM_BOOK3S_H__ */
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index de686b340f4a..363c2fb0d89e 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1816,7 +1816,7 @@ static int threads_per_vcore(struct kvm *kvm)
> return threads_per_subcore;
> }
>
> -static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
> +static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int id)
> {
> struct kvmppc_vcore *vcore;
>
> @@ -1830,7 +1830,7 @@ static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
> init_swait_queue_head(&vcore->wq);
> vcore->preempt_tb = TB_NIL;
> vcore->lpcr = kvm->arch.lpcr;
> - vcore->first_vcpuid = core * kvm->arch.smt_mode;
> + vcore->first_vcpuid = id;
> vcore->kvm = kvm;
> INIT_LIST_HEAD(&vcore->preempt_list);
>
> @@ -2048,12 +2048,18 @@ static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct kvm *kvm,
> mutex_lock(&kvm->lock);
> vcore = NULL;
> err = -EINVAL;
> - core = id / kvm->arch.smt_mode;
> + if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> + BUG_ON(kvm->arch.smt_mode != 1);
> + core = kvmppc_pack_vcpu_id(kvm, id);
> + } else {
> + core = id / kvm->arch.smt_mode;
> + }
> if (core < KVM_MAX_VCORES) {
> vcore = kvm->arch.vcores[core];
> + BUG_ON(cpu_has_feature(CPU_FTR_ARCH_300) && vcore);
> if (!vcore) {
> err = -ENOMEM;
> - vcore = kvmppc_vcore_create(kvm, core);
> + vcore = kvmppc_vcore_create(kvm, id & ~(kvm->arch.smt_mode - 1));
> kvm->arch.vcores[core] = vcore;
> kvm->arch.online_vcores++;
> }
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index f9818d7d3381..dbd5887daf4a 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -317,6 +317,11 @@ static int xive_select_target(struct kvm *kvm, u32 *server, u8 prio)
> return -EBUSY;
> }
>
> +static u32 xive_vp(struct kvmppc_xive *xive, u32 server)
> +{
> + return xive->vp_base + kvmppc_pack_vcpu_id(xive->kvm, server);
> +}
> +
> static u8 xive_lock_and_mask(struct kvmppc_xive *xive,
> struct kvmppc_xive_src_block *sb,
> struct kvmppc_xive_irq_state *state)
> @@ -362,7 +367,7 @@ static u8 xive_lock_and_mask(struct kvmppc_xive *xive,
> */
> if (xd->flags & OPAL_XIVE_IRQ_MASK_VIA_FW) {
> xive_native_configure_irq(hw_num,
> - xive->vp_base + state->act_server,
> + xive_vp(xive, state->act_server),
> MASKED, state->number);
> /* set old_p so we can track if an H_EOI was done */
> state->old_p = true;
> @@ -418,7 +423,7 @@ static void xive_finish_unmask(struct kvmppc_xive *xive,
> */
> if (xd->flags & OPAL_XIVE_IRQ_MASK_VIA_FW) {
> xive_native_configure_irq(hw_num,
> - xive->vp_base + state->act_server,
> + xive_vp(xive, state->act_server),
> state->act_priority, state->number);
> /* If an EOI is needed, do it here */
> if (!state->old_p)
> @@ -495,7 +500,7 @@ static int xive_target_interrupt(struct kvm *kvm,
> kvmppc_xive_select_irq(state, &hw_num, NULL);
>
> return xive_native_configure_irq(hw_num,
> - xive->vp_base + server,
> + xive_vp(xive, server),
> prio, state->number);
> }
>
> @@ -883,7 +888,7 @@ int kvmppc_xive_set_mapped(struct kvm *kvm, unsigned long guest_irq,
> * which is fine for a never started interrupt.
> */
> xive_native_configure_irq(hw_irq,
> - xive->vp_base + state->act_server,
> + xive_vp(xive, state->act_server),
> state->act_priority, state->number);
>
> /*
> @@ -959,7 +964,7 @@ int kvmppc_xive_clr_mapped(struct kvm *kvm, unsigned long guest_irq,
>
> /* Reconfigure the IPI */
> xive_native_configure_irq(state->ipi_number,
> - xive->vp_base + state->act_server,
> + xive_vp(xive, state->act_server),
> state->act_priority, state->number);
>
> /*
> @@ -1084,7 +1089,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
> pr_devel("Duplicate !\n");
> return -EEXIST;
> }
> - if (cpu >= KVM_MAX_VCPUS) {
> + if (cpu >= KVM_MAX_VCPU_ID) {
> pr_devel("Out of bounds !\n");
> return -EINVAL;
> }
> @@ -1098,7 +1103,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
> xc->xive = xive;
> xc->vcpu = vcpu;
> xc->server_num = cpu;
> - xc->vp_id = xive->vp_base + cpu;
> + xc->vp_id = xive_vp(xive, cpu);
> xc->mfrr = 0xff;
> xc->valid = true;
>
>
^ permalink raw reply
* Re: [PATCH v3 1/1] KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
From: Cédric Le Goater @ 2018-07-24 9:07 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev; +Cc: paulus, kvm-ppc, kvm, david
In-Reply-To: <1fb3aea5f44f1029866ee10db40abde7e18b24ad.1531967105.git.sbobroff@linux.ibm.com>
On 07/19/2018 04:25 AM, Sam Bobroff wrote:
> From: Sam Bobroff <sam.bobroff@au1.ibm.com>
>
> It is not currently possible to create the full number of possible
> VCPUs (KVM_MAX_VCPUS) on Power9 with KVM-HV when the guest uses less
> threads per core than it's core stride (or "VSMT mode"). This is
> because the VCORE ID and XIVE offsets to grow beyond KVM_MAX_VCPUS
> even though the VCPU ID is less than KVM_MAX_VCPU_ID.
>
> To address this, "pack" the VCORE ID and XIVE offsets by using
> knowledge of the way the VCPU IDs will be used when there are less
> guest threads per core than the core stride. The primary thread of
> each core will always be used first. Then, if the guest uses more than
> one thread per core, these secondary threads will sequentially follow
> the primary in each core.
>
> So, the only way an ID above KVM_MAX_VCPUS can be seen, is if the
> VCPUs are being spaced apart, so at least half of each core is empty
> and IDs between KVM_MAX_VCPUS and (KVM_MAX_VCPUS * 2) can be mapped
> into the second half of each core (4..7, in an 8-thread core).
>
> Similarly, if IDs above KVM_MAX_VCPUS * 2 are seen, at least 3/4 of
> each core is being left empty, and we can map down into the second and
> third quarters of each core (2, 3 and 5, 6 in an 8-thread core).
>
> Lastly, if IDs above KVM_MAX_VCPUS * 4 are seen, only the primary
> threads are being used and 7/8 of the core is empty, allowing use of
> the 1, 3, 5 and 7 thread slots.
>
> (Strides less than 8 are handled similarly.)
>
> This allows the VCORE ID or offset to be calculated quickly from the
> VCPU ID or XIVE server numbers, without access to the VCPU structure.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
On the XIVE part,
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> Hello everyone,
>
> I've completed a trial merge with the guest native-XIVE code and found no
> problems; it's no more difficult than the host side and only requires a few
> calls to xive_vp().
>
> On that basis, here is v3 (unchanged from v2) as non-RFC and it seems to be
> ready to go.
>
> Patch set v3:
> Patch 1/1: KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
>
> Patch set v2:
> Patch 1/1: KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
> * Corrected places in kvm/book3s_xive.c where IDs weren't packed.
> * Because kvmppc_pack_vcpu_id() is only called on P9, there is no need to test "emul_smt_mode > 1", so remove it.
> * Re-ordered block_offsets[] to be more ascending.
> * Added more detailed description of the packing algorithm.
>
> Patch set v1:
> Patch 1/1: KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
>
> arch/powerpc/include/asm/kvm_book3s.h | 44 +++++++++++++++++++++++++++++++++++
> arch/powerpc/kvm/book3s_hv.c | 14 +++++++----
> arch/powerpc/kvm/book3s_xive.c | 19 +++++++++------
> 3 files changed, 66 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> index 1f345a0b6ba2..ba4b6e00fca7 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -390,4 +390,48 @@ extern int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu);
> #define SPLIT_HACK_MASK 0xff000000
> #define SPLIT_HACK_OFFS 0xfb000000
>
> +/* Pack a VCPU ID from the [0..KVM_MAX_VCPU_ID) space down to the
> + * [0..KVM_MAX_VCPUS) space, while using knowledge of the guest's core stride
> + * (but not it's actual threading mode, which is not available) to avoid
> + * collisions.
> + *
> + * The implementation leaves VCPU IDs from the range [0..KVM_MAX_VCPUS) (block
> + * 0) unchanged: if the guest is filling each VCORE completely then it will be
> + * using consecutive IDs and it will fill the space without any packing.
> + *
> + * For higher VCPU IDs, the packed ID is based on the VCPU ID modulo
> + * KVM_MAX_VCPUS (effectively masking off the top bits) and then an offset is
> + * added to avoid collisions.
> + *
> + * VCPU IDs in the range [KVM_MAX_VCPUS..(KVM_MAX_VCPUS*2)) (block 1) are only
> + * possible if the guest is leaving at least 1/2 of each VCORE empty, so IDs
> + * can be safely packed into the second half of each VCORE by adding an offset
> + * of (stride / 2).
> + *
> + * Similarly, if VCPU IDs in the range [(KVM_MAX_VCPUS*2)..(KVM_MAX_VCPUS*4))
> + * (blocks 2 and 3) are seen, the guest must be leaving at least 3/4 of each
> + * VCORE empty so packed IDs can be offset by (stride / 4) and (stride * 3 / 4).
> + *
> + * Finally, VCPU IDs from blocks 5..7 will only be seen if the guest is using a
> + * stride of 8 and 1 thread per core so the remaining offsets of 1, 3, 5 and 7
> + * must be free to use.
> + *
> + * (The offsets for each block are stored in block_offsets[], indexed by the
> + * block number if the stride is 8. For cases where the guest's stride is less
> + * than 8, we can re-use the block_offsets array by multiplying the block
> + * number by (MAX_SMT_THREADS / stride) to reach the correct entry.)
> + */
> +static inline u32 kvmppc_pack_vcpu_id(struct kvm *kvm, u32 id)
> +{
> + const int block_offsets[MAX_SMT_THREADS] = {0, 4, 2, 6, 1, 3, 5, 7};
> + int stride = kvm->arch.emul_smt_mode;
> + int block = (id / KVM_MAX_VCPUS) * (MAX_SMT_THREADS / stride);
> + u32 packed_id;
> +
> + BUG_ON(block >= MAX_SMT_THREADS);
> + packed_id = (id % KVM_MAX_VCPUS) + block_offsets[block];
> + BUG_ON(packed_id >= KVM_MAX_VCPUS);
> + return packed_id;
> +}
> +
> #endif /* __ASM_KVM_BOOK3S_H__ */
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index de686b340f4a..363c2fb0d89e 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1816,7 +1816,7 @@ static int threads_per_vcore(struct kvm *kvm)
> return threads_per_subcore;
> }
>
> -static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
> +static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int id)
> {
> struct kvmppc_vcore *vcore;
>
> @@ -1830,7 +1830,7 @@ static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
> init_swait_queue_head(&vcore->wq);
> vcore->preempt_tb = TB_NIL;
> vcore->lpcr = kvm->arch.lpcr;
> - vcore->first_vcpuid = core * kvm->arch.smt_mode;
> + vcore->first_vcpuid = id;
> vcore->kvm = kvm;
> INIT_LIST_HEAD(&vcore->preempt_list);
>
> @@ -2048,12 +2048,18 @@ static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct kvm *kvm,
> mutex_lock(&kvm->lock);
> vcore = NULL;
> err = -EINVAL;
> - core = id / kvm->arch.smt_mode;
> + if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> + BUG_ON(kvm->arch.smt_mode != 1);
> + core = kvmppc_pack_vcpu_id(kvm, id);
> + } else {
> + core = id / kvm->arch.smt_mode;
> + }
> if (core < KVM_MAX_VCORES) {
> vcore = kvm->arch.vcores[core];
> + BUG_ON(cpu_has_feature(CPU_FTR_ARCH_300) && vcore);
> if (!vcore) {
> err = -ENOMEM;
> - vcore = kvmppc_vcore_create(kvm, core);
> + vcore = kvmppc_vcore_create(kvm, id & ~(kvm->arch.smt_mode - 1));
> kvm->arch.vcores[core] = vcore;
> kvm->arch.online_vcores++;
> }
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index f9818d7d3381..dbd5887daf4a 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -317,6 +317,11 @@ static int xive_select_target(struct kvm *kvm, u32 *server, u8 prio)
> return -EBUSY;
> }
>
> +static u32 xive_vp(struct kvmppc_xive *xive, u32 server)
> +{
> + return xive->vp_base + kvmppc_pack_vcpu_id(xive->kvm, server);
> +}
> +
> static u8 xive_lock_and_mask(struct kvmppc_xive *xive,
> struct kvmppc_xive_src_block *sb,
> struct kvmppc_xive_irq_state *state)
> @@ -362,7 +367,7 @@ static u8 xive_lock_and_mask(struct kvmppc_xive *xive,
> */
> if (xd->flags & OPAL_XIVE_IRQ_MASK_VIA_FW) {
> xive_native_configure_irq(hw_num,
> - xive->vp_base + state->act_server,
> + xive_vp(xive, state->act_server),
> MASKED, state->number);
> /* set old_p so we can track if an H_EOI was done */
> state->old_p = true;
> @@ -418,7 +423,7 @@ static void xive_finish_unmask(struct kvmppc_xive *xive,
> */
> if (xd->flags & OPAL_XIVE_IRQ_MASK_VIA_FW) {
> xive_native_configure_irq(hw_num,
> - xive->vp_base + state->act_server,
> + xive_vp(xive, state->act_server),
> state->act_priority, state->number);
> /* If an EOI is needed, do it here */
> if (!state->old_p)
> @@ -495,7 +500,7 @@ static int xive_target_interrupt(struct kvm *kvm,
> kvmppc_xive_select_irq(state, &hw_num, NULL);
>
> return xive_native_configure_irq(hw_num,
> - xive->vp_base + server,
> + xive_vp(xive, server),
> prio, state->number);
> }
>
> @@ -883,7 +888,7 @@ int kvmppc_xive_set_mapped(struct kvm *kvm, unsigned long guest_irq,
> * which is fine for a never started interrupt.
> */
> xive_native_configure_irq(hw_irq,
> - xive->vp_base + state->act_server,
> + xive_vp(xive, state->act_server),
> state->act_priority, state->number);
>
> /*
> @@ -959,7 +964,7 @@ int kvmppc_xive_clr_mapped(struct kvm *kvm, unsigned long guest_irq,
>
> /* Reconfigure the IPI */
> xive_native_configure_irq(state->ipi_number,
> - xive->vp_base + state->act_server,
> + xive_vp(xive, state->act_server),
> state->act_priority, state->number);
>
> /*
> @@ -1084,7 +1089,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
> pr_devel("Duplicate !\n");
> return -EEXIST;
> }
> - if (cpu >= KVM_MAX_VCPUS) {
> + if (cpu >= KVM_MAX_VCPU_ID) {
> pr_devel("Out of bounds !\n");
> return -EINVAL;
> }
> @@ -1098,7 +1103,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
> xc->xive = xive;
> xc->vcpu = vcpu;
> xc->server_num = cpu;
> - xc->vp_id = xive->vp_base + cpu;
> + xc->vp_id = xive_vp(xive, cpu);
> xc->mfrr = 0xff;
> xc->valid = true;
>
>
^ permalink raw reply
* Re: [PATCH] sched/numa: do not balance tasks onto isolated cpus
From: Peter Zijlstra @ 2018-07-24 9:06 UTC (permalink / raw)
To: jiang.biao2
Cc: chen.lin130, mingo, linux-kernel, zhong.weidong, tan.hu,
cheng.lin130, tan.hu
In-Reply-To: <201807240911372861670@zte.com.cn>
On Tue, Jul 24, 2018 at 09:11:37AM +0800, jiang.biao2@zte.com.cn wrote:
> >> Signed-off-by: Cheng Lin <cheng.lin130@zte.com.cn>
> >> Signed-off-by: Tan Hu <tan.hu@zte.com.cn>
> >> Signed-off-by: Jiang Biao <jiang.biao2@zte.com.cn>
> >
> >This SoB chain is invalid.
> Mm, we don't quite understand what the *Signed-off-by* precisely means,
> Does it only mean DCO(developer certificate of origin)?
> As we understood, multiple SoBs could be used to indicate co-authors.
> If SoB only means DCO, how can the patches reflect co-authors?
It specifically does not allow for co-authorship. I think there's a
Co-Authored-by: tag invented by some people (check the git logs) but
especially for such a dinky little patch, I wouldn't bother. Maybe have
your co-workers review the patch or something.
^ permalink raw reply
* Re: [PATCH v6 5/5] mailbox: Add support for i.MX7D messaging unit
From: Lucas Stach @ 2018-07-24 9:06 UTC (permalink / raw)
To: Oleksij Rempel, Shawn Guo, Fabio Estevam, Rob Herring,
Mark Rutland, A.s. Dong, Vladimir Zapolskiy
Cc: kernel, devicetree, dl-linux-imx, linux-arm-kernel
In-Reply-To: <eebba930-b3d7-3670-3d96-35cc6c2a6bcb@pengutronix.de>
Am Dienstag, den 24.07.2018, 07:14 +0200 schrieb Oleksij Rempel:
> Hi Lucas,
>
> here is more detailed response:
>
> On 23.07.2018 19:19, Lucas Stach wrote:
[...]
> > > +};
> > > +
> > > +static struct imx_mu_priv *to_imx_mu_priv(struct mbox_controller *mbox)
> > > +{
> > > > + return container_of(mbox, struct imx_mu_priv, mbox);
> > >
> > > +}
> > > +
> > > +static void imx_mu_write(struct imx_mu_priv *priv, u32 val, u32 offs)
> > > +{
> > > + iowrite32(val, priv->base + offs);
> >
> > This driver is never going to be used on a device with port based IO,
> > so iowrite doesn't make much sense here, just use writel. Same comment
> > applies to the below read function.
>
> As before I don't really understand the difference. I took some time for
> learning and here are my findings:
> - historical background of ioread/iowrite functions. What I can find is
> > this LWN article: https://lwn.net/Articles/102232/ . The main point
> seems to be "The first of these is a new __iomem annotation used to mark
> pointers to I/O memory" ... " As with __user, the __iomem marker serves
> a documentation role in the kernel code;"
>
> In modern kernel the difference between ioread32 and readl seems to be
> completely disappeared.
> with this LWN article (2016):
> https://lwn.net/Articles/698014/
> the readl and reade32 are always in the same group..
Okay, as discussed offline this is more a thing of personal preference,
since this maps to the same thing internally when used on a
architecture without IO ports. I slightly dislike those as they don't
have a _relaxed counterpart, but other than that there should be no
difference. As we don't use any of the relaxed stuff in this driver
it's really about the color of the bikeshed, so feel free to keep it
your way.
> If there is some documentation which will say why I should use readl()
> instead of ioread32() i would really love to read it.
>
> > Also, given that those functions are not really shortening the code in
> > the user they may also be removed completely IMHO.
>
> Wrong assumption.. I use this functions for tracing. It is just to easy
> to add two trace points... From technical perspective I don't see any
> advantage or disadvantage of having this functions.
> If it is my personal preference, then I decide to keep it.
That IMHO implies that I'm perfectly fine with you ignoring this
comment. :)
[...]
> > > +static int imx_mu_startup(struct mbox_chan *chan)
> > > +{
> > > > > > > > + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > > > > > > > + struct imx_mu_con_priv *cp = chan->con_priv;
> > > > + int ret;
> > >
> > > +
> > > > > > > > + cp->irq_desc = devm_kasprintf(priv->dev, GFP_KERNEL, "imx_mu_chan[%i]",
> > > > > > > > + cp->idx);
> > > > > > > > + if (!cp->irq_desc)
> > > > + return -ENOMEM;
> > >
> > > +
> > > > + ret = devm_request_irq(priv->dev, cp->irq, imx_mu_isr,
> > >
> > > + IRQF_SHARED, cp->irq_desc, chan);
> >
> > Using the devm_ variants of those functions doesn't make sense when the
> > resources aren't tied to the device lifetime. As you are tearing them
> > down manually in imx_mu_shutdown anyways, just use the raw variants of
> > those functions.
>
> I have nothing against it.
Thanks, as this is something I feel more strongly about. As the devm_
stuff is meant to tie the lifetime of an object to the device/driver
lifetime using them in a context where there is no relation at all is
kind of an API abuse to me.
Regards,
Lucas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v6 5/5] mailbox: Add support for i.MX7D messaging unit
From: Lucas Stach @ 2018-07-24 9:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <eebba930-b3d7-3670-3d96-35cc6c2a6bcb@pengutronix.de>
Am Dienstag, den 24.07.2018, 07:14 +0200 schrieb Oleksij Rempel:
> Hi Lucas,
>
> here is more detailed response:
>
> On 23.07.2018 19:19, Lucas Stach wrote:
[...]
> > > +};
> > > +
> > > +static struct imx_mu_priv *to_imx_mu_priv(struct mbox_controller *mbox)
> > > +{
> > > > + return container_of(mbox, struct imx_mu_priv, mbox);
> > >
> > > +}
> > > +
> > > +static void imx_mu_write(struct imx_mu_priv *priv, u32 val, u32 offs)
> > > +{
> > > + iowrite32(val, priv->base + offs);
> >
> > This driver is never going to be used on a device with port based IO,
> > so iowrite doesn't make much sense here, just use writel. Same comment
> > applies to the below read function.
>
> As before I don't really understand the difference. I took some time for
> learning and here are my findings:
> - historical background of ioread/iowrite functions. What I can find is
> > this LWN article: https://lwn.net/Articles/102232/ . The main point
> seems to be "The first of these is a new __iomem annotation used to mark
> pointers to I/O memory" ... " As with __user, the __iomem marker serves
> a documentation role in the kernel code;"
>
> In modern kernel the difference between ioread32 and readl seems to be
> completely disappeared.
> with this LWN article (2016):
> https://lwn.net/Articles/698014/
> the readl and reade32 are always in the same group..
Okay, as discussed offline this is more a thing of personal preference,
since this maps to the same thing internally when used on a
architecture without IO ports. I slightly dislike those as they don't
have a _relaxed counterpart, but other than that there should be no
difference. As we don't use any of the relaxed stuff in this driver
it's really about the color of the bikeshed, so feel free to keep it
your way.
> If there is some documentation which will say why I should use readl()
> instead of ioread32() i would really love to read it.
>
> > Also, given that those functions are not really shortening the code in
> > the user they may also be removed completely IMHO.
>
> Wrong assumption..??I use this functions for tracing. It is just to easy
> to add two trace points...??From technical perspective I don't see any
> advantage or disadvantage of having this functions.
> If it is my personal preference, then I decide to keep it.
That IMHO implies that I'm perfectly fine with you ignoring this
comment. :)
[...]
> > > +static int imx_mu_startup(struct mbox_chan *chan)
> > > +{
> > > > > > > > + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > > > > > > > + struct imx_mu_con_priv *cp = chan->con_priv;
> > > > + int ret;
> > >
> > > +
> > > > > > > > + cp->irq_desc = devm_kasprintf(priv->dev, GFP_KERNEL, "imx_mu_chan[%i]",
> > > > > > > > + ??????cp->idx);
> > > > > > > > + if (!cp->irq_desc)
> > > > + return -ENOMEM;
> > >
> > > +
> > > > + ret = devm_request_irq(priv->dev, cp->irq, imx_mu_isr,
> > >
> > > + ???????IRQF_SHARED, cp->irq_desc, chan);
> >
> > Using the devm_ variants of those functions doesn't make sense when the
> > resources aren't tied to the device lifetime. As you are tearing them
> > down manually in imx_mu_shutdown anyways, just use the raw variants of
> > those functions.
>
> I have nothing against it.
Thanks, as this is something I feel more strongly about. As the devm_
stuff is meant to tie the lifetime of an object to the device/driver
lifetime using them in a context where there is no relation at all is
kind of an API abuse to me.
Regards,
Lucas
^ permalink raw reply
* Re: automation: Creating a patchwork instance to improve pre-commit build testing
From: Jan Beulich @ 2018-07-24 9:06 UTC (permalink / raw)
To: Doug Goldstein, Lars Kurth, Wei Liu
Cc: Minios-devel, xen-devel, Iurii Artemenko, committers,
Matt Spencer
In-Reply-To: <DD622B30-B54C-4B9A-AA24-05329BD800B9@citrix.com>
>>> On 23.07.18 at 18:40, <lars.kurth@citrix.com> wrote:
> # How does this impact me?
> The contribution workflow is *not* impacted by this change, but once up and
> running the following will happen once you post a patch or patch series to
> xen-devel:
> * Patchwork will take patch series from the mailing list and applies it
> * CI/DC testing is triggered
> * A test report will be sent as a mail to the patch or the series (aka the 00 patch of the series)
>
> This does mean though that series which do not build or show other issues,
> will likely not be reviewed until the tests pass. This would lessen the
> burden on reviewers, as they will know whether the code submitted builds on a
> wide array of environments.
So how are dependencies between series intended to be dealt with? It
is not uncommon for someone to say "applies only on top of xyz". The
implication of "will likely not be reviewed until the tests pass" seems
unsuitable to me in such a case.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* [v3,23/23] usb: usbtmc: Remove sysfs group TermChar and auto_abort
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
As all the properties of the usbtmc driver can now be
controlled on a per file descriptor basis by ioctl functions
the sysfs interface is of limited use.
We are not aware about applications that are using the sysfs
parameter TermChar, TermCharEnabled or auto_abort.
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
.../ABI/stable/sysfs-driver-usb-usbtmc | 35 --------
drivers/usb/class/usbtmc.c | 84 +------------------
2 files changed, 3 insertions(+), 116 deletions(-)
diff --git a/Documentation/ABI/stable/sysfs-driver-usb-usbtmc b/Documentation/ABI/stable/sysfs-driver-usb-usbtmc
index e960cd027e1e..a9e123ba32cd 100644
--- a/Documentation/ABI/stable/sysfs-driver-usb-usbtmc
+++ b/Documentation/ABI/stable/sysfs-driver-usb-usbtmc
@@ -25,38 +25,3 @@ Description:
4.2.2.
The files are read only.
-
-
-What: /sys/bus/usb/drivers/usbtmc/*/TermChar
-Date: August 2008
-Contact: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-Description:
- This file is the TermChar value to be sent to the USB TMC
- device as described by the document, "Universal Serial Bus Test
- and Measurement Class Specification
- (USBTMC) Revision 1.0" as published by the USB-IF.
-
- Note that the TermCharEnabled file determines if this value is
- sent to the device or not.
-
-
-What: /sys/bus/usb/drivers/usbtmc/*/TermCharEnabled
-Date: August 2008
-Contact: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-Description:
- This file determines if the TermChar is to be sent to the
- device on every transaction or not. For more details about
- this, please see the document, "Universal Serial Bus Test and
- Measurement Class Specification (USBTMC) Revision 1.0" as
- published by the USB-IF.
-
-
-What: /sys/bus/usb/drivers/usbtmc/*/auto_abort
-Date: August 2008
-Contact: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-Description:
- This file determines if the transaction of the USB TMC
- device is to be automatically aborted if there is any error.
- For more details about this, please see the document,
- "Universal Serial Bus Test and Measurement Class Specification
- (USBTMC) Revision 1.0" as published by the USB-IF.
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 9a6312a49b55..f8c7dab6d786 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -102,11 +102,6 @@ struct usbtmc_device_data {
/* coalesced usb488_caps from usbtmc_dev_capabilities */
__u8 usb488_caps;
- /* attributes from the USB TMC spec for this device */
- u8 TermChar;
- bool TermCharEnabled;
- bool auto_abort;
-
bool zombie; /* fd of disconnected device */
struct usbtmc_dev_capabilities capabilities;
@@ -211,11 +206,10 @@ static int usbtmc_open(struct inode *inode, struct file *filp)
atomic_set(&file_data->closing, 0);
- /* copy default values from device settings */
file_data->timeout = USBTMC_TIMEOUT;
- file_data->term_char = data->TermChar;
- file_data->term_char_enabled = data->TermCharEnabled;
- file_data->auto_abort = data->auto_abort;
+ file_data->term_char = '\n';
+ file_data->term_char_enabled = 0;
+ file_data->auto_abort = 0;
file_data->eom_val = 1;
INIT_LIST_HEAD(&file_data->file_elem);
@@ -1866,72 +1860,6 @@ static const struct attribute_group capability_attr_grp = {
.attrs = capability_attrs,
};
-static ssize_t TermChar_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct usb_interface *intf = to_usb_interface(dev);
- struct usbtmc_device_data *data = usb_get_intfdata(intf);
-
- return sprintf(buf, "%c\n", data->TermChar);
-}
-
-static ssize_t TermChar_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct usb_interface *intf = to_usb_interface(dev);
- struct usbtmc_device_data *data = usb_get_intfdata(intf);
-
- if (count < 1)
- return -EINVAL;
- data->TermChar = buf[0];
- return count;
-}
-static DEVICE_ATTR_RW(TermChar);
-
-#define data_attribute(name) \
-static ssize_t name##_show(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct usb_interface *intf = to_usb_interface(dev); \
- struct usbtmc_device_data *data = usb_get_intfdata(intf); \
- \
- return sprintf(buf, "%d\n", data->name); \
-} \
-static ssize_t name##_store(struct device *dev, \
- struct device_attribute *attr, \
- const char *buf, size_t count) \
-{ \
- struct usb_interface *intf = to_usb_interface(dev); \
- struct usbtmc_device_data *data = usb_get_intfdata(intf); \
- ssize_t result; \
- unsigned val; \
- \
- result = sscanf(buf, "%u\n", &val); \
- if (result != 1) \
- result = -EINVAL; \
- data->name = val; \
- if (result < 0) \
- return result; \
- else \
- return count; \
-} \
-static DEVICE_ATTR_RW(name)
-
-data_attribute(TermCharEnabled);
-data_attribute(auto_abort);
-
-static struct attribute *data_attrs[] = {
- &dev_attr_TermChar.attr,
- &dev_attr_TermCharEnabled.attr,
- &dev_attr_auto_abort.attr,
- NULL,
-};
-
-static const struct attribute_group data_attr_grp = {
- .attrs = data_attrs,
-};
-
static int usbtmc_ioctl_indicator_pulse(struct usbtmc_device_data *data)
{
struct device *dev;
@@ -2435,8 +2363,6 @@ static int usbtmc_probe(struct usb_interface *intf,
/* Initialize USBTMC bTag and other fields */
data->bTag = 1;
- data->TermCharEnabled = 0;
- data->TermChar = '\n';
/* 2 <= bTag <= 127 USBTMC-USB488 subclass specification 4.3.1 */
data->iin_bTag = 2;
@@ -2510,8 +2436,6 @@ static int usbtmc_probe(struct usb_interface *intf,
}
}
- retcode = sysfs_create_group(&intf->dev.kobj, &data_attr_grp);
-
retcode = usb_register_dev(intf, &usbtmc_class);
if (retcode) {
dev_err(&intf->dev, "Not able to get a minor (base %u, slice default): %d\n",
@@ -2525,7 +2449,6 @@ static int usbtmc_probe(struct usb_interface *intf,
error_register:
sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp);
- sysfs_remove_group(&intf->dev.kobj, &data_attr_grp);
usbtmc_free_int(data);
err_put:
kref_put(&data->kref, usbtmc_delete);
@@ -2539,7 +2462,6 @@ static void usbtmc_disconnect(struct usb_interface *intf)
usb_deregister_dev(intf, &usbtmc_class);
sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp);
- sysfs_remove_group(&intf->dev.kobj, &data_attr_grp);
mutex_lock(&data->io_mutex);
data->zombie = 1;
wake_up_interruptible_all(&data->waitq);
^ permalink raw reply related
* [v3,22/23] usb: usbtmc: Fix split quoted string in debug message
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index ddc2e4b78a3b..9a6312a49b55 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -2514,8 +2514,8 @@ static int usbtmc_probe(struct usb_interface *intf,
retcode = usb_register_dev(intf, &usbtmc_class);
if (retcode) {
- dev_err(&intf->dev, "Not able to get a minor"
- " (base %u, slice default): %d\n", USBTMC_MINOR_BASE,
+ dev_err(&intf->dev, "Not able to get a minor (base %u, slice default): %d\n",
+ USBTMC_MINOR_BASE,
retcode);
goto error_register;
}
^ permalink raw reply related
* [v3,21/23] usb: usbtmc: Remove redundant macro USBTMC_SIZE_IOBUFFER
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index a6482e850532..ddc2e4b78a3b 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -30,12 +30,6 @@
#define USBTMC_HEADER_SIZE 12
#define USBTMC_MINOR_BASE 176
-/*
- * Size of driver internal IO buffer. Must be multiple of 4 and at least as
- * large as wMaxPacketSize (which is usually 512 bytes).
- */
-#define USBTMC_SIZE_IOBUFFER 2048
-
/* Minimum USB timeout (in milliseconds) */
#define USBTMC_MIN_TIMEOUT 100
/* Default USB timeout (in milliseconds) */
^ permalink raw reply related
* [v3,20/23] usb: usbtmc: Remove redundant code
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Remove redundant code and fix debug messages.
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 10dc896bf043..a6482e850532 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -1760,12 +1760,9 @@ static int usbtmc_ioctl_clear_out_halt(struct usbtmc_device_data *data)
rv = usb_clear_halt(data->usb_dev,
usb_sndbulkpipe(data->usb_dev, data->bulk_out));
- if (rv < 0) {
- dev_err(&data->usb_dev->dev, "usb_control_msg returned %d\n",
- rv);
- return rv;
- }
- return 0;
+ if (rv < 0)
+ dev_err(&data->usb_dev->dev, "%s returned %d\n", __func__, rv);
+ return rv;
}
static int usbtmc_ioctl_clear_in_halt(struct usbtmc_device_data *data)
@@ -1775,12 +1772,9 @@ static int usbtmc_ioctl_clear_in_halt(struct usbtmc_device_data *data)
rv = usb_clear_halt(data->usb_dev,
usb_rcvbulkpipe(data->usb_dev, data->bulk_in));
- if (rv < 0) {
- dev_err(&data->usb_dev->dev, "usb_control_msg returned %d\n",
- rv);
- return rv;
- }
- return 0;
+ if (rv < 0)
+ dev_err(&data->usb_dev->dev, "%s returned %d\n", __func__, rv);
+ return rv;
}
static int usbtmc_ioctl_cancel_io(struct usbtmc_file_data *file_data)
@@ -2204,11 +2198,8 @@ static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
break;
case USBTMC488_IOCTL_GET_CAPS:
- retval = copy_to_user((void __user *)arg,
- &data->usb488_caps,
- sizeof(data->usb488_caps));
- if (retval)
- retval = -EFAULT;
+ retval = put_user(data->usb488_caps,
+ (unsigned char __user *)arg);
break;
case USBTMC488_IOCTL_READ_STB:
^ permalink raw reply related
* [v3,19/23] usb: usbtmc: Update ioctl-number.txt
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Reserve a suitable range of ioctl numbers for USBTMC driver.
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
Documentation/ioctl/ioctl-number.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 480c8609dc58..810dd6c30296 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -200,7 +200,7 @@ Code Seq#(hex) Include File Comments
'X' 01 linux/pktcdvd.h conflict!
'Y' all linux/cyclades.h
'Z' 14-15 drivers/message/fusion/mptctl.h
-'[' 00-07 linux/usb/tmc.h USB Test and Measurement Devices
+'[' 00-3F linux/usb/tmc.h USB Test and Measurement Devices
<mailto:gregkh@linuxfoundation.org>
'a' all linux/atm*.h, linux/sonet.h ATM on linux
<http://lrcwww.epfl.ch/>
^ permalink raw reply related
* [v3,18/23] usb: usbtmc: Add ioctl USBTMC_IOCTL_API_VERSION
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Add ioctl USBTMC_IOCTL_API_VERSION to get current API version
of usbtmc driver.
This is to allow an instrument library to determine whether
the driver API is compatible with the implementation.
The API may change in future versions. Therefore the macro
USBTMC_API_VERSION should be incremented when changing tmc.h
with new flags, ioctls or when changing a significant behavior
of the driver.
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 9 +++++++++
include/uapi/linux/usb/tmc.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 1d0ac45346c5..10dc896bf043 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -22,6 +22,10 @@
#include <linux/compat.h>
#include <linux/usb/tmc.h>
+/* Increment API VERSION when changing tmc.h with new flags or ioctls
+ * or when changing a significant behavior of the driver.
+ */
+#define USBTMC_API_VERSION (2)
#define USBTMC_HEADER_SIZE 12
#define USBTMC_MINOR_BASE 176
@@ -2194,6 +2198,11 @@ static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
(void __user *)arg);
break;
+ case USBTMC_IOCTL_API_VERSION:
+ retval = put_user(USBTMC_API_VERSION,
+ (__u32 __user *)arg);
+ break;
+
case USBTMC488_IOCTL_GET_CAPS:
retval = copy_to_user((void __user *)arg,
&data->usb488_caps,
diff --git a/include/uapi/linux/usb/tmc.h b/include/uapi/linux/usb/tmc.h
index 2435694d3ba5..1d3373922e5e 100644
--- a/include/uapi/linux/usb/tmc.h
+++ b/include/uapi/linux/usb/tmc.h
@@ -89,6 +89,7 @@ struct usbtmc_message {
#define USBTMC_IOCTL_WRITE _IOWR(USBTMC_IOC_NR, 13, struct usbtmc_message)
#define USBTMC_IOCTL_READ _IOWR(USBTMC_IOC_NR, 14, struct usbtmc_message)
#define USBTMC_IOCTL_WRITE_RESULT _IOWR(USBTMC_IOC_NR, 15, __u32)
+#define USBTMC_IOCTL_API_VERSION _IOR(USBTMC_IOC_NR, 16, __u32)
#define USBTMC488_IOCTL_GET_CAPS _IOR(USBTMC_IOC_NR, 17, unsigned char)
#define USBTMC488_IOCTL_READ_STB _IOR(USBTMC_IOC_NR, 18, unsigned char)
^ permalink raw reply related
* [v3,17/23] usb: usbtmc: Replace USBTMC_TIMEOUT macros for control messages
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Use common timeout macro USB_CTRL_GET_TIMEOUT (=5s) for all
usb_control_msg() function calls.
The macro USBTMC_TIMEOUT should only be used as default value for
Bulk IN/OUT transfers.
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 5dad62c7a996..1d0ac45346c5 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -536,7 +536,7 @@ static int usbtmc488_ioctl_read_stb(struct usbtmc_file_data *file_data,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
data->iin_bTag,
data->ifnum,
- buffer, 0x03, USBTMC_TIMEOUT);
+ buffer, 0x03, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "stb usb_control_msg returned %d\n", rv);
goto exit;
@@ -670,7 +670,7 @@ static int usbtmc488_ioctl_simple(struct usbtmc_device_data *data,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
wValue,
data->ifnum,
- buffer, 0x01, USBTMC_TIMEOUT);
+ buffer, 0x01, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "simple usb_control_msg failed %d\n", rv);
goto exit;
@@ -1817,7 +1817,7 @@ static int get_capabilities(struct usbtmc_device_data *data)
rv = usb_control_msg(data->usb_dev, usb_rcvctrlpipe(data->usb_dev, 0),
USBTMC_REQUEST_GET_CAPABILITIES,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
- 0, 0, buffer, 0x18, USBTMC_TIMEOUT);
+ 0, 0, buffer, 0x18, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "usb_control_msg returned %d\n", rv);
goto err_out;
@@ -1956,7 +1956,7 @@ static int usbtmc_ioctl_indicator_pulse(struct usbtmc_device_data *data)
usb_rcvctrlpipe(data->usb_dev, 0),
USBTMC_REQUEST_INDICATOR_PULSE,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
- 0, 0, buffer, 0x01, USBTMC_TIMEOUT);
+ 0, 0, buffer, 0x01, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "usb_control_msg returned %d\n", rv);
^ permalink raw reply related
* [v3,16/23] usb: usbtmc: Fix ioctl USBTMC_IOCTL_ABORT_BULK_OUT
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Add parameter 'tag' to function usbtmc_ioctl_abort_bulk_out_tag()
for future versions.
Use USBTMC_BUFSIZE (4k) instead of USBTMC_SIZE_IOBUFFER (2k).
Using USBTMC_SIZE_IOBUFFER is deprecated.
Insert a sleep of 50 ms between subsequent
CHECK_ABORT_BULK_OUT_STATUS control requests to avoid stressing
the instrument with repeated requests.
Use common macro USB_CTRL_GET_TIMEOUT instead of USBTMC_TIMEOUT.
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index e05c11ba26f6..5dad62c7a996 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -413,7 +413,8 @@ static int usbtmc_ioctl_abort_bulk_in(struct usbtmc_device_data *data)
return usbtmc_ioctl_abort_bulk_in_tag(data, data->bTag_last_read);
}
-static int usbtmc_ioctl_abort_bulk_out(struct usbtmc_device_data *data)
+static int usbtmc_ioctl_abort_bulk_out_tag(struct usbtmc_device_data *data,
+ u8 tag)
{
struct device *dev;
u8 *buffer;
@@ -430,8 +431,8 @@ static int usbtmc_ioctl_abort_bulk_out(struct usbtmc_device_data *data)
usb_rcvctrlpipe(data->usb_dev, 0),
USBTMC_REQUEST_INITIATE_ABORT_BULK_OUT,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_ENDPOINT,
- data->bTag_last_write, data->bulk_out,
- buffer, 2, USBTMC_TIMEOUT);
+ tag, data->bulk_out,
+ buffer, 2, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "usb_control_msg returned %d\n", rv);
@@ -450,12 +451,14 @@ static int usbtmc_ioctl_abort_bulk_out(struct usbtmc_device_data *data)
n = 0;
usbtmc_abort_bulk_out_check_status:
+ /* do not stress device with subsequent requests */
+ msleep(50);
rv = usb_control_msg(data->usb_dev,
usb_rcvctrlpipe(data->usb_dev, 0),
USBTMC_REQUEST_CHECK_ABORT_BULK_OUT_STATUS,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_ENDPOINT,
0, data->bulk_out, buffer, 0x08,
- USBTMC_TIMEOUT);
+ USB_CTRL_GET_TIMEOUT);
n++;
if (rv < 0) {
dev_err(dev, "usb_control_msg returned %d\n", rv);
@@ -489,6 +492,11 @@ static int usbtmc_ioctl_abort_bulk_out(struct usbtmc_device_data *data)
return rv;
}
+static int usbtmc_ioctl_abort_bulk_out(struct usbtmc_device_data *data)
+{
+ return usbtmc_ioctl_abort_bulk_out_tag(data, data->bTag_last_write);
+}
+
static int usbtmc488_ioctl_read_stb(struct usbtmc_file_data *file_data,
void __user *arg)
{
^ permalink raw reply related
* [v3,15/23] usb: usbtmc: Fix ioctl USBTMC_IOCTL_ABORT_BULK_IN
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Add parameter 'tag' to function usbtmc_ioctl_abort_bulk_in_tag()
for future versions.
Remove calculation of max_size (=wMaxPacketSize) and wrong
condition (actual == max_size) in while loop. An abort operation
should always flush the complete Bulk-IN until a short packet is
received.
Return error code ENOMSG when transfer (specified by given tag)
is not in progress and device returns code
USBTMC_STATUS_TRANSFER_NOT_IN_PROGRESS.
Use USBTMC_BUFSIZE (4k) instead of USBTMC_SIZE_IOBUFFER (2k).
Using USBTMC_SIZE_IOBUFFER is deprecated.
Use common macro USB_CTRL_GET_TIMEOUT instead of USBTMC_TIMEOUT.
Check only bit 0 (field bmAbortBulkIn) of the
CHECK_ABORT_BULK_IN_STATUS response, since other bits are reserved
and can change in future versions.
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 139 ++++++++++++++++---------------------
1 file changed, 61 insertions(+), 78 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index a7fa9e287477..e05c11ba26f6 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -285,18 +285,17 @@ static int usbtmc_release(struct inode *inode, struct file *file)
return 0;
}
-static int usbtmc_ioctl_abort_bulk_in(struct usbtmc_device_data *data)
+static int usbtmc_ioctl_abort_bulk_in_tag(struct usbtmc_device_data *data,
+ u8 tag)
{
u8 *buffer;
struct device *dev;
int rv;
int n;
int actual;
- struct usb_host_interface *current_setting;
- int max_size;
dev = &data->intf->dev;
- buffer = kmalloc(USBTMC_SIZE_IOBUFFER, GFP_KERNEL);
+ buffer = kmalloc(USBTMC_BUFSIZE, GFP_KERNEL);
if (!buffer)
return -ENOMEM;
@@ -304,21 +303,34 @@ static int usbtmc_ioctl_abort_bulk_in(struct usbtmc_device_data *data)
usb_rcvctrlpipe(data->usb_dev, 0),
USBTMC_REQUEST_INITIATE_ABORT_BULK_IN,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_ENDPOINT,
- data->bTag_last_read, data->bulk_in,
- buffer, 2, USBTMC_TIMEOUT);
+ tag, data->bulk_in,
+ buffer, 2, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "usb_control_msg returned %d\n", rv);
goto exit;
}
- dev_dbg(dev, "INITIATE_ABORT_BULK_IN returned %x\n", buffer[0]);
+ dev_dbg(dev, "INITIATE_ABORT_BULK_IN returned %x with tag %02x\n",
+ buffer[0], buffer[1]);
if (buffer[0] == USBTMC_STATUS_FAILED) {
+ /* No transfer in progress and the Bulk-OUT FIFO is empty. */
rv = 0;
goto exit;
}
+ if (buffer[0] == USBTMC_STATUS_TRANSFER_NOT_IN_PROGRESS) {
+ /* The device returns this status if either:
+ * - There is a transfer in progress, but the specified bTag
+ * does not match.
+ * - There is no transfer in progress, but the Bulk-OUT FIFO
+ * is not empty.
+ */
+ rv = -ENOMSG;
+ goto exit;
+ }
+
if (buffer[0] != USBTMC_STATUS_SUCCESS) {
dev_err(dev, "INITIATE_ABORT_BULK_IN returned %x\n",
buffer[0]);
@@ -326,64 +338,52 @@ static int usbtmc_ioctl_abort_bulk_in(struct usbtmc_device_data *data)
goto exit;
}
- max_size = 0;
- current_setting = data->intf->cur_altsetting;
- for (n = 0; n < current_setting->desc.bNumEndpoints; n++)
- if (current_setting->endpoint[n].desc.bEndpointAddress ==
- data->bulk_in)
- max_size = usb_endpoint_maxp(¤t_setting->endpoint[n].desc);
-
- if (max_size == 0) {
- dev_err(dev, "Couldn't get wMaxPacketSize\n");
- rv = -EPERM;
- goto exit;
- }
-
- dev_dbg(&data->intf->dev, "wMaxPacketSize is %d\n", max_size);
-
- n = 0;
-
- do {
- dev_dbg(dev, "Reading from bulk in EP\n");
-
- rv = usb_bulk_msg(data->usb_dev,
- usb_rcvbulkpipe(data->usb_dev,
- data->bulk_in),
- buffer, USBTMC_SIZE_IOBUFFER,
- &actual, USBTMC_TIMEOUT);
-
- n++;
-
- if (rv < 0) {
- dev_err(dev, "usb_bulk_msg returned %d\n", rv);
- goto exit;
- }
- } while ((actual == max_size) &&
- (n < USBTMC_MAX_READS_TO_CLEAR_BULK_IN));
-
- if (actual == max_size) {
- dev_err(dev, "Couldn't clear device buffer within %d cycles\n",
- USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
- rv = -EPERM;
- goto exit;
- }
-
n = 0;
usbtmc_abort_bulk_in_status:
+ dev_dbg(dev, "Reading from bulk in EP\n");
+
+ /* Data must be present. So use low timeout 300 ms */
+ rv = usb_bulk_msg(data->usb_dev,
+ usb_rcvbulkpipe(data->usb_dev,
+ data->bulk_in),
+ buffer, USBTMC_BUFSIZE,
+ &actual, 300);
+
+ print_hex_dump_debug("usbtmc ", DUMP_PREFIX_NONE, 16, 1,
+ buffer, actual, true);
+
+ n++;
+
+ if (rv < 0) {
+ dev_err(dev, "usb_bulk_msg returned %d\n", rv);
+ if (rv != -ETIMEDOUT)
+ goto exit;
+ }
+
+ if (actual == USBTMC_BUFSIZE)
+ goto usbtmc_abort_bulk_in_status;
+
+ if (n >= USBTMC_MAX_READS_TO_CLEAR_BULK_IN) {
+ dev_err(dev, "Couldn't clear device buffer within %d cycles\n",
+ USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
+ rv = -EPERM;
+ goto exit;
+ }
+
rv = usb_control_msg(data->usb_dev,
usb_rcvctrlpipe(data->usb_dev, 0),
USBTMC_REQUEST_CHECK_ABORT_BULK_IN_STATUS,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_ENDPOINT,
0, data->bulk_in, buffer, 0x08,
- USBTMC_TIMEOUT);
+ USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "usb_control_msg returned %d\n", rv);
goto exit;
}
- dev_dbg(dev, "INITIATE_ABORT_BULK_IN returned %x\n", buffer[0]);
+ dev_dbg(dev, "CHECK_ABORT_BULK_IN returned %x\n", buffer[0]);
if (buffer[0] == USBTMC_STATUS_SUCCESS) {
rv = 0;
@@ -391,43 +391,26 @@ static int usbtmc_ioctl_abort_bulk_in(struct usbtmc_device_data *data)
}
if (buffer[0] != USBTMC_STATUS_PENDING) {
- dev_err(dev, "INITIATE_ABORT_BULK_IN returned %x\n", buffer[0]);
+ dev_err(dev, "CHECK_ABORT_BULK_IN returned %x\n", buffer[0]);
rv = -EPERM;
goto exit;
}
- if (buffer[1] == 1)
- do {
- dev_dbg(dev, "Reading from bulk in EP\n");
-
- rv = usb_bulk_msg(data->usb_dev,
- usb_rcvbulkpipe(data->usb_dev,
- data->bulk_in),
- buffer, USBTMC_SIZE_IOBUFFER,
- &actual, USBTMC_TIMEOUT);
-
- n++;
-
- if (rv < 0) {
- dev_err(dev, "usb_bulk_msg returned %d\n", rv);
- goto exit;
- }
- } while ((actual == max_size) &&
- (n < USBTMC_MAX_READS_TO_CLEAR_BULK_IN));
-
- if (actual == max_size) {
- dev_err(dev, "Couldn't clear device buffer within %d cycles\n",
- USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
- rv = -EPERM;
- goto exit;
+ if ((buffer[1] & 1) > 0) {
+ /* The device has 1 or more queued packets the Host can read */
+ goto usbtmc_abort_bulk_in_status;
}
- goto usbtmc_abort_bulk_in_status;
-
+ /* The Host must send CHECK_ABORT_BULK_IN_STATUS at a later time. */
+ rv = -EAGAIN;
exit:
kfree(buffer);
return rv;
+}
+static int usbtmc_ioctl_abort_bulk_in(struct usbtmc_device_data *data)
+{
+ return usbtmc_ioctl_abort_bulk_in_tag(data, data->bTag_last_read);
}
static int usbtmc_ioctl_abort_bulk_out(struct usbtmc_device_data *data)
^ permalink raw reply related
* [v3,14/23] usb: usbtmc: Fix ioctl USBTMC_IOCTL_CLEAR
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Remove calculation of max_size (=wMaxPacketSize) and wrong
condition (actual == max_size) in while loop. A device clear
should always flush the complete Bulk-IN FIFO.
Insert a sleep of 50 ms between subsequent CHECK_CLEAR_STATUS
control requests to avoid stressing the instrument with
repeated requests.
Some instruments need time to cleanup internal I/O buffers.
Polling and nonbraked requests slow down the response time of
devices.
Use USBTMC_BUFSIZE (4k) instead of USBTMC_SIZE_IOBUFFER (2k).
Using USBTMC_SIZE_IOBUFFER is deprecated.
Check only bit 0 (field bmClear) of the CHECK_CLEAR_STATUS
response, since other bits are reserved and can change in
future versions.
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 46 +++++++++++++++-----------------------
1 file changed, 18 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index e5ea2d822ae3..a7fa9e287477 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -1647,20 +1647,17 @@ static ssize_t usbtmc_write(struct file *filp, const char __user *buf,
static int usbtmc_ioctl_clear(struct usbtmc_device_data *data)
{
- struct usb_host_interface *current_setting;
- struct usb_endpoint_descriptor *desc;
struct device *dev;
u8 *buffer;
int rv;
int n;
int actual = 0;
- int max_size;
dev = &data->intf->dev;
dev_dbg(dev, "Sending INITIATE_CLEAR request\n");
- buffer = kmalloc(USBTMC_SIZE_IOBUFFER, GFP_KERNEL);
+ buffer = kmalloc(USBTMC_BUFSIZE, GFP_KERNEL);
if (!buffer)
return -ENOMEM;
@@ -1668,7 +1665,7 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data *data)
usb_rcvctrlpipe(data->usb_dev, 0),
USBTMC_REQUEST_INITIATE_CLEAR,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
- 0, 0, buffer, 1, USBTMC_TIMEOUT);
+ 0, 0, buffer, 1, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "usb_control_msg returned %d\n", rv);
goto exit;
@@ -1682,22 +1679,6 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data *data)
goto exit;
}
- max_size = 0;
- current_setting = data->intf->cur_altsetting;
- for (n = 0; n < current_setting->desc.bNumEndpoints; n++) {
- desc = ¤t_setting->endpoint[n].desc;
- if (desc->bEndpointAddress == data->bulk_in)
- max_size = usb_endpoint_maxp(desc);
- }
-
- if (max_size == 0) {
- dev_err(dev, "Couldn't get wMaxPacketSize\n");
- rv = -EPERM;
- goto exit;
- }
-
- dev_dbg(dev, "wMaxPacketSize is %d\n", max_size);
-
n = 0;
usbtmc_clear_check_status:
@@ -1708,7 +1689,7 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data *data)
usb_rcvctrlpipe(data->usb_dev, 0),
USBTMC_REQUEST_CHECK_CLEAR_STATUS,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
- 0, 0, buffer, 2, USBTMC_TIMEOUT);
+ 0, 0, buffer, 2, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "usb_control_msg returned %d\n", rv);
goto exit;
@@ -1725,15 +1706,19 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data *data)
goto exit;
}
- if (buffer[1] == 1)
+ if ((buffer[1] & 1) != 0) {
do {
dev_dbg(dev, "Reading from bulk in EP\n");
rv = usb_bulk_msg(data->usb_dev,
usb_rcvbulkpipe(data->usb_dev,
data->bulk_in),
- buffer, USBTMC_SIZE_IOBUFFER,
- &actual, USBTMC_TIMEOUT);
+ buffer, USBTMC_BUFSIZE,
+ &actual, USB_CTRL_GET_TIMEOUT);
+
+ print_hex_dump_debug("usbtmc ", DUMP_PREFIX_NONE,
+ 16, 1, buffer, actual, true);
+
n++;
if (rv < 0) {
@@ -1741,10 +1726,15 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data *data)
rv);
goto exit;
}
- } while ((actual == max_size) &&
+ } while ((actual == USBTMC_BUFSIZE) &&
(n < USBTMC_MAX_READS_TO_CLEAR_BULK_IN));
+ } else {
+ /* do not stress device with subsequent requests */
+ msleep(50);
+ n++;
+ }
- if (actual == max_size) {
+ if (n >= USBTMC_MAX_READS_TO_CLEAR_BULK_IN) {
dev_err(dev, "Couldn't clear device buffer within %d cycles\n",
USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
rv = -EPERM;
@@ -1758,7 +1748,7 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data *data)
rv = usb_clear_halt(data->usb_dev,
usb_sndbulkpipe(data->usb_dev, data->bulk_out));
if (rv < 0) {
- dev_err(dev, "usb_control_msg returned %d\n", rv);
+ dev_err(dev, "usb_clear_halt returned %d\n", rv);
goto exit;
}
rv = 0;
^ permalink raw reply related
* [v3,13/23] usb: usbtmc: Optimize usbtmc_read
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Use new usbtmc_generic_read function to maximize bandwidth
during long data transfer. Also fix reading of zero length
packet (ZLP) or trailing short packet.
The maximum input transfer size is limited to INT_MAX (=2GB).
Also remove redundant return in send_request_dev_dep_msg_in().
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 222 ++++++++++++++++++-------------------
1 file changed, 105 insertions(+), 117 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 26d5af413009..e5ea2d822ae3 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -1308,7 +1308,7 @@ static ssize_t usbtmc_ioctl_write_result(struct usbtmc_file_data *file_data,
* Also updates bTag_last_write.
*/
static int send_request_dev_dep_msg_in(struct usbtmc_file_data *file_data,
- size_t transfer_size)
+ u32 transfer_size)
{
struct usbtmc_device_data *data = file_data->data;
int retval;
@@ -1351,12 +1351,11 @@ static int send_request_dev_dep_msg_in(struct usbtmc_file_data *file_data,
data->bTag++;
kfree(buffer);
- if (retval < 0) {
- dev_err(&data->intf->dev, "usb_bulk_msg in send_request_dev_dep_msg_in() returned %d\n", retval);
- return retval;
- }
+ if (retval < 0)
+ dev_err(&data->intf->dev, "%s returned %d\n",
+ __func__, retval);
- return 0;
+ return retval;
}
static ssize_t usbtmc_read(struct file *filp, char __user *buf,
@@ -1365,20 +1364,20 @@ static ssize_t usbtmc_read(struct file *filp, char __user *buf,
struct usbtmc_file_data *file_data;
struct usbtmc_device_data *data;
struct device *dev;
+ const u32 bufsize = USBTMC_BUFSIZE;
u32 n_characters;
u8 *buffer;
int actual;
- size_t done;
- size_t remaining;
+ u32 done = 0;
+ u32 remaining;
int retval;
- size_t this_part;
/* Get pointer to private data structure */
file_data = filp->private_data;
data = file_data->data;
dev = &data->intf->dev;
- buffer = kmalloc(USBTMC_SIZE_IOBUFFER, GFP_KERNEL);
+ buffer = kmalloc(bufsize, GFP_KERNEL);
if (!buffer)
return -ENOMEM;
@@ -1388,7 +1387,10 @@ static ssize_t usbtmc_read(struct file *filp, char __user *buf,
goto exit;
}
- dev_dbg(dev, "usb_bulk_msg_in: count(%zu)\n", count);
+ if (count > INT_MAX)
+ count = INT_MAX;
+
+ dev_dbg(dev, "%s(count:%zu)\n", __func__, count);
retval = send_request_dev_dep_msg_in(file_data, count);
@@ -1400,114 +1402,100 @@ static ssize_t usbtmc_read(struct file *filp, char __user *buf,
/* Loop until we have fetched everything we requested */
remaining = count;
- this_part = remaining;
- done = 0;
-
- while (remaining > 0) {
- /* Send bulk URB */
- retval = usb_bulk_msg(data->usb_dev,
- usb_rcvbulkpipe(data->usb_dev,
- data->bulk_in),
- buffer, USBTMC_SIZE_IOBUFFER, &actual,
- file_data->timeout);
-
- dev_dbg(dev, "usb_bulk_msg: retval(%u), done(%zu), remaining(%zu), actual(%d)\n", retval, done, remaining, actual);
-
- /* Store bTag (in case we need to abort) */
- data->bTag_last_read = data->bTag;
-
- if (retval < 0) {
- dev_dbg(dev, "Unable to read data, error %d\n", retval);
- if (file_data->auto_abort)
- usbtmc_ioctl_abort_bulk_in(data);
+
+ /* Send bulk URB */
+ retval = usb_bulk_msg(data->usb_dev,
+ usb_rcvbulkpipe(data->usb_dev,
+ data->bulk_in),
+ buffer, bufsize, &actual,
+ file_data->timeout);
+
+ dev_dbg(dev, "%s: bulk_msg retval(%u), actual(%d)\n",
+ __func__, retval, actual);
+
+ /* Store bTag (in case we need to abort) */
+ data->bTag_last_read = data->bTag;
+
+ if (retval < 0) {
+ if (file_data->auto_abort)
+ usbtmc_ioctl_abort_bulk_in(data);
+ goto exit;
+ }
+
+ /* Sanity checks for the header */
+ if (actual < USBTMC_HEADER_SIZE) {
+ dev_err(dev, "Device sent too small first packet: %u < %u\n",
+ actual, USBTMC_HEADER_SIZE);
+ if (file_data->auto_abort)
+ usbtmc_ioctl_abort_bulk_in(data);
+ goto exit;
+ }
+
+ if (buffer[0] != 2) {
+ dev_err(dev, "Device sent reply with wrong MsgID: %u != 2\n",
+ buffer[0]);
+ if (file_data->auto_abort)
+ usbtmc_ioctl_abort_bulk_in(data);
+ goto exit;
+ }
+
+ if (buffer[1] != data->bTag_last_write) {
+ dev_err(dev, "Device sent reply with wrong bTag: %u != %u\n",
+ buffer[1], data->bTag_last_write);
+ if (file_data->auto_abort)
+ usbtmc_ioctl_abort_bulk_in(data);
+ goto exit;
+ }
+
+ /* How many characters did the instrument send? */
+ n_characters = buffer[4] +
+ (buffer[5] << 8) +
+ (buffer[6] << 16) +
+ (buffer[7] << 24);
+
+ file_data->bmTransferAttributes = buffer[8];
+
+ dev_dbg(dev, "Bulk-IN header: N_characters(%u), bTransAttr(%u)\n",
+ n_characters, buffer[8]);
+
+ if (n_characters > remaining) {
+ dev_err(dev, "Device wants to return more data than requested: %u > %zu\n",
+ n_characters, count);
+ if (file_data->auto_abort)
+ usbtmc_ioctl_abort_bulk_in(data);
+ goto exit;
+ }
+
+ print_hex_dump_debug("usbtmc ", DUMP_PREFIX_NONE,
+ 16, 1, buffer, actual, true);
+
+ remaining = n_characters;
+
+ /* Remove the USBTMC header */
+ actual -= USBTMC_HEADER_SIZE;
+
+ /* Remove padding if it exists */
+ if (actual > remaining)
+ actual = remaining;
+
+ remaining -= actual;
+
+ /* Copy buffer to user space */
+ if (copy_to_user(buf, &buffer[USBTMC_HEADER_SIZE], actual)) {
+ /* There must have been an addressing problem */
+ retval = -EFAULT;
+ goto exit;
+ }
+
+ if ((actual + USBTMC_HEADER_SIZE) == bufsize) {
+ retval = usbtmc_generic_read(file_data, buf + actual,
+ remaining,
+ &done,
+ USBTMC_FLAG_IGNORE_TRAILER);
+ if (retval < 0)
goto exit;
- }
-
- /* Parse header in first packet */
- if (done == 0) {
- /* Sanity checks for the header */
- if (actual < USBTMC_HEADER_SIZE) {
- dev_err(dev, "Device sent too small first packet: %u < %u\n", actual, USBTMC_HEADER_SIZE);
- if (file_data->auto_abort)
- usbtmc_ioctl_abort_bulk_in(data);
- goto exit;
- }
-
- if (buffer[0] != 2) {
- dev_err(dev, "Device sent reply with wrong MsgID: %u != 2\n", buffer[0]);
- if (file_data->auto_abort)
- usbtmc_ioctl_abort_bulk_in(data);
- goto exit;
- }
-
- if (buffer[1] != data->bTag_last_write) {
- dev_err(dev, "Device sent reply with wrong bTag: %u != %u\n", buffer[1], data->bTag_last_write);
- if (file_data->auto_abort)
- usbtmc_ioctl_abort_bulk_in(data);
- goto exit;
- }
-
- /* How many characters did the instrument send? */
- n_characters = buffer[4] +
- (buffer[5] << 8) +
- (buffer[6] << 16) +
- (buffer[7] << 24);
-
- file_data->bmTransferAttributes = buffer[8];
-
- if (n_characters > this_part) {
- dev_err(dev, "Device wants to return more data than requested: %u > %zu\n", n_characters, count);
- if (file_data->auto_abort)
- usbtmc_ioctl_abort_bulk_in(data);
- goto exit;
- }
-
- /* Remove the USBTMC header */
- actual -= USBTMC_HEADER_SIZE;
-
- /* Check if the message is smaller than requested */
- if (remaining > n_characters)
- remaining = n_characters;
- /* Remove padding if it exists */
- if (actual > remaining)
- actual = remaining;
-
- dev_dbg(dev, "Bulk-IN header: N_characters(%u), bTransAttr(%u)\n", n_characters, buffer[8]);
-
- remaining -= actual;
-
- /* Terminate if end-of-message bit received from device */
- if ((buffer[8] & 0x01) && (actual >= n_characters))
- remaining = 0;
-
- dev_dbg(dev, "Bulk-IN header: remaining(%zu), buf(%p), buffer(%p) done(%zu)\n", remaining,buf,buffer,done);
-
-
- /* Copy buffer to user space */
- if (copy_to_user(buf + done, &buffer[USBTMC_HEADER_SIZE], actual)) {
- /* There must have been an addressing problem */
- retval = -EFAULT;
- goto exit;
- }
- done += actual;
- }
- else {
- if (actual > remaining)
- actual = remaining;
-
- remaining -= actual;
-
- dev_dbg(dev, "Bulk-IN header cont: actual(%u), done(%zu), remaining(%zu), buf(%p), buffer(%p)\n", actual, done, remaining,buf,buffer);
-
- /* Copy buffer to user space */
- if (copy_to_user(buf + done, buffer, actual)) {
- /* There must have been an addressing problem */
- retval = -EFAULT;
- goto exit;
- }
- done += actual;
- }
}
+ done += actual;
/* Update file position value */
*f_pos = *f_pos + done;
^ permalink raw reply related
* [v3,12/23] usb: usbtmc: Optimize usbtmc_write
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Use new usbtmc_generic_write function to maximize bandwidth
during long data transfer.
The maximum output transfer size is limited to INT_MAX (=2GB).
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 176 +++++++++++++++++++++++--------------
1 file changed, 109 insertions(+), 67 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index caadd2df13a9..26d5af413009 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -1524,94 +1524,136 @@ static ssize_t usbtmc_write(struct file *filp, const char __user *buf,
{
struct usbtmc_file_data *file_data;
struct usbtmc_device_data *data;
+ struct urb *urb = NULL;
+ ssize_t retval = 0;
u8 *buffer;
- int retval;
- int actual;
- unsigned long int n_bytes;
- int remaining;
- int done;
- int this_part;
+ u32 remaining, done;
+ u32 transfersize, aligned, buflen;
file_data = filp->private_data;
data = file_data->data;
- buffer = kmalloc(USBTMC_SIZE_IOBUFFER, GFP_KERNEL);
- if (!buffer)
- return -ENOMEM;
-
mutex_lock(&data->io_mutex);
+
if (data->zombie) {
retval = -ENODEV;
goto exit;
}
- remaining = count;
done = 0;
- while (remaining > 0) {
- if (remaining > USBTMC_SIZE_IOBUFFER - USBTMC_HEADER_SIZE) {
- this_part = USBTMC_SIZE_IOBUFFER - USBTMC_HEADER_SIZE;
- buffer[8] = 0;
- } else {
- this_part = remaining;
- buffer[8] = file_data->eom_val;
- }
-
- /* Setup IO buffer for DEV_DEP_MSG_OUT message */
- buffer[0] = 1;
- buffer[1] = data->bTag;
- buffer[2] = ~data->bTag;
- buffer[3] = 0; /* Reserved */
- buffer[4] = this_part >> 0;
- buffer[5] = this_part >> 8;
- buffer[6] = this_part >> 16;
- buffer[7] = this_part >> 24;
- /* buffer[8] is set above... */
- buffer[9] = 0; /* Reserved */
- buffer[10] = 0; /* Reserved */
- buffer[11] = 0; /* Reserved */
-
- if (copy_from_user(&buffer[USBTMC_HEADER_SIZE], buf + done, this_part)) {
- retval = -EFAULT;
- goto exit;
- }
-
- n_bytes = roundup(USBTMC_HEADER_SIZE + this_part, 4);
- memset(buffer + USBTMC_HEADER_SIZE + this_part, 0, n_bytes - (USBTMC_HEADER_SIZE + this_part));
-
- do {
- retval = usb_bulk_msg(data->usb_dev,
- usb_sndbulkpipe(data->usb_dev,
- data->bulk_out),
- buffer, n_bytes,
- &actual, file_data->timeout);
- if (retval != 0)
- break;
- n_bytes -= actual;
- } while (n_bytes);
-
- data->bTag_last_write = data->bTag;
+ spin_lock_irq(&file_data->err_lock);
+ file_data->out_transfer_size = 0;
+ file_data->out_status = 0;
+ spin_unlock_irq(&file_data->err_lock);
+
+ if (!count)
+ goto exit;
+
+ if (down_trylock(&file_data->limit_write_sem)) {
+ /* previous calls were async */
+ retval = -EBUSY;
+ goto exit;
+ }
+
+ urb = usbtmc_create_urb();
+ if (!urb) {
+ retval = -ENOMEM;
+ up(&file_data->limit_write_sem);
+ goto exit;
+ }
+
+ buffer = urb->transfer_buffer;
+ buflen = urb->transfer_buffer_length;
+
+ if (count > INT_MAX) {
+ transfersize = INT_MAX;
+ buffer[8] = 0;
+ } else {
+ transfersize = count;
+ buffer[8] = file_data->eom_val;
+ }
+
+ /* Setup IO buffer for DEV_DEP_MSG_OUT message */
+ buffer[0] = 1;
+ buffer[1] = data->bTag;
+ buffer[2] = ~data->bTag;
+ buffer[3] = 0; /* Reserved */
+ buffer[4] = transfersize >> 0;
+ buffer[5] = transfersize >> 8;
+ buffer[6] = transfersize >> 16;
+ buffer[7] = transfersize >> 24;
+ /* buffer[8] is set above... */
+ buffer[9] = 0; /* Reserved */
+ buffer[10] = 0; /* Reserved */
+ buffer[11] = 0; /* Reserved */
+
+ remaining = transfersize;
+
+ if (transfersize + USBTMC_HEADER_SIZE > buflen) {
+ transfersize = buflen - USBTMC_HEADER_SIZE;
+ aligned = buflen;
+ } else {
+ aligned = (transfersize + (USBTMC_HEADER_SIZE + 3)) & ~3;
+ }
+
+ if (copy_from_user(&buffer[USBTMC_HEADER_SIZE], buf, transfersize)) {
+ retval = -EFAULT;
+ up(&file_data->limit_write_sem);
+ goto exit;
+ }
+
+ dev_dbg(&data->intf->dev, "%s(size:%u align:%u)\n", __func__,
+ (unsigned int)transfersize, (unsigned int)aligned);
+
+ print_hex_dump_debug("usbtmc ", DUMP_PREFIX_NONE,
+ 16, 1, buffer, aligned, true);
+
+ usb_fill_bulk_urb(urb, data->usb_dev,
+ usb_sndbulkpipe(data->usb_dev, data->bulk_out),
+ urb->transfer_buffer, aligned,
+ usbtmc_write_bulk_cb, file_data);
+
+ usb_anchor_urb(urb, &file_data->submitted);
+ retval = usb_submit_urb(urb, GFP_KERNEL);
+ if (unlikely(retval)) {
+ usb_unanchor_urb(urb);
+ up(&file_data->limit_write_sem);
+ goto exit;
+ }
+
+ remaining -= transfersize;
+
+ data->bTag_last_write = data->bTag;
+ data->bTag++;
+
+ if (!data->bTag)
data->bTag++;
- if (!data->bTag)
- data->bTag++;
+ /* call generic_write even when remaining = 0 */
+ retval = usbtmc_generic_write(file_data, buf + transfersize, remaining,
+ &done, USBTMC_FLAG_APPEND);
+ /* truncate alignment bytes */
+ if (done > remaining)
+ done = remaining;
- if (retval < 0) {
- dev_err(&data->intf->dev,
- "Unable to send data, error %d\n", retval);
- if (file_data->auto_abort)
- usbtmc_ioctl_abort_bulk_out(data);
- goto exit;
- }
+ /*add size of first urb*/
+ done += transfersize;
- remaining -= this_part;
- done += this_part;
+ if (retval < 0) {
+ usb_kill_anchored_urbs(&file_data->submitted);
+
+ dev_err(&data->intf->dev,
+ "Unable to send data, error %d\n", (int)retval);
+ if (file_data->auto_abort)
+ usbtmc_ioctl_abort_bulk_out(data);
+ goto exit;
}
- retval = count;
+ retval = done;
exit:
+ usb_free_urb(urb);
mutex_unlock(&data->io_mutex);
- kfree(buffer);
return retval;
}
^ permalink raw reply related
* [v3,11/23] usb: usbtmc: Add ioctl USBTMC_IOCTL_AUTO_ABORT
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Add ioctl USBTMC_IOCTL_AUTO_ABORT to configure auto_abort for
each specific file handle.
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 23 ++++++++++++++++-------
include/uapi/linux/usb/tmc.h | 1 +
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index d659c083a19b..caadd2df13a9 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -136,6 +136,7 @@ struct usbtmc_file_data {
u8 eom_val;
u8 term_char;
bool term_char_enabled;
+ bool auto_abort;
spinlock_t err_lock; /* lock for errors */
@@ -216,6 +217,7 @@ static int usbtmc_open(struct inode *inode, struct file *filp)
file_data->timeout = USBTMC_TIMEOUT;
file_data->term_char = data->TermChar;
file_data->term_char_enabled = data->TermCharEnabled;
+ file_data->auto_abort = data->auto_abort;
file_data->eom_val = 1;
INIT_LIST_HEAD(&file_data->file_elem);
@@ -1391,7 +1393,7 @@ static ssize_t usbtmc_read(struct file *filp, char __user *buf,
retval = send_request_dev_dep_msg_in(file_data, count);
if (retval < 0) {
- if (data->auto_abort)
+ if (file_data->auto_abort)
usbtmc_ioctl_abort_bulk_out(data);
goto exit;
}
@@ -1416,7 +1418,7 @@ static ssize_t usbtmc_read(struct file *filp, char __user *buf,
if (retval < 0) {
dev_dbg(dev, "Unable to read data, error %d\n", retval);
- if (data->auto_abort)
+ if (file_data->auto_abort)
usbtmc_ioctl_abort_bulk_in(data);
goto exit;
}
@@ -1426,21 +1428,21 @@ static ssize_t usbtmc_read(struct file *filp, char __user *buf,
/* Sanity checks for the header */
if (actual < USBTMC_HEADER_SIZE) {
dev_err(dev, "Device sent too small first packet: %u < %u\n", actual, USBTMC_HEADER_SIZE);
- if (data->auto_abort)
+ if (file_data->auto_abort)
usbtmc_ioctl_abort_bulk_in(data);
goto exit;
}
if (buffer[0] != 2) {
dev_err(dev, "Device sent reply with wrong MsgID: %u != 2\n", buffer[0]);
- if (data->auto_abort)
+ if (file_data->auto_abort)
usbtmc_ioctl_abort_bulk_in(data);
goto exit;
}
if (buffer[1] != data->bTag_last_write) {
dev_err(dev, "Device sent reply with wrong bTag: %u != %u\n", buffer[1], data->bTag_last_write);
- if (data->auto_abort)
+ if (file_data->auto_abort)
usbtmc_ioctl_abort_bulk_in(data);
goto exit;
}
@@ -1455,7 +1457,7 @@ static ssize_t usbtmc_read(struct file *filp, char __user *buf,
if (n_characters > this_part) {
dev_err(dev, "Device wants to return more data than requested: %u > %zu\n", n_characters, count);
- if (data->auto_abort)
+ if (file_data->auto_abort)
usbtmc_ioctl_abort_bulk_in(data);
goto exit;
}
@@ -1597,7 +1599,7 @@ static ssize_t usbtmc_write(struct file *filp, const char __user *buf,
if (retval < 0) {
dev_err(&data->intf->dev,
"Unable to send data, error %d\n", retval);
- if (data->auto_abort)
+ if (file_data->auto_abort)
usbtmc_ioctl_abort_bulk_out(data);
goto exit;
}
@@ -2106,6 +2108,7 @@ static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct usbtmc_file_data *file_data;
struct usbtmc_device_data *data;
int retval = -EBADRQC;
+ __u8 tmp_byte;
file_data = file->private_data;
data = file_data->data;
@@ -2222,6 +2225,12 @@ static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
(__u8 __user *)arg);
break;
+ case USBTMC_IOCTL_AUTO_ABORT:
+ retval = get_user(tmp_byte, (unsigned char __user *)arg);
+ if (retval == 0)
+ file_data->auto_abort = !!tmp_byte;
+ break;
+
case USBTMC_IOCTL_CANCEL_IO:
retval = usbtmc_ioctl_cancel_io(file_data);
break;
diff --git a/include/uapi/linux/usb/tmc.h b/include/uapi/linux/usb/tmc.h
index d2ed6a20de6b..2435694d3ba5 100644
--- a/include/uapi/linux/usb/tmc.h
+++ b/include/uapi/linux/usb/tmc.h
@@ -99,6 +99,7 @@ struct usbtmc_message {
#define USBTMC488_IOCTL_WAIT_SRQ _IOW(USBTMC_IOC_NR, 23, __u32)
#define USBTMC_IOCTL_MSG_IN_ATTR _IOR(USBTMC_IOC_NR, 24, __u8)
+#define USBTMC_IOCTL_AUTO_ABORT _IOW(USBTMC_IOC_NR, 25, __u8)
/* Cancel and cleanup asynchronous calls */
#define USBTMC_IOCTL_CANCEL_IO _IO(USBTMC_IOC_NR, 35)
^ permalink raw reply related
* [v3,10/23] usb: usbtmc: add ioctl USBTMC_IOCTL_MSG_IN_ATTR
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
add ioctl USBTMC_IOCTL_MSG_IN_ATTR that returns the specific
bmTransferAttributes field of the last DEV_DEP_MSG_IN Bulk-IN
header. This header is received by the read() function. The
meaning of the (u8) bitmap bmTransferAttributes is:
Bit 0 = EOM flag is set when the last transfer of a USBTMC
message is received.
Bit 1 = is set when the last byte is a termchar (e.g. '\n').
Note that this bit is always zero when the device does not support
the termchar feature or when termchar detection is not enabled
(see ioctl USBTMC_IOCTL_CONFIG_TERMCHAR).
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 8 ++++++++
include/uapi/linux/usb/tmc.h | 2 ++
2 files changed, 10 insertions(+)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index edce0a753623..d659c083a19b 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -131,6 +131,7 @@ struct usbtmc_file_data {
u8 srq_byte;
atomic_t srq_asserted;
atomic_t closing;
+ u8 bmTransferAttributes; /* member of DEV_DEP_MSG_IN */
u8 eom_val;
u8 term_char;
@@ -1450,6 +1451,8 @@ static ssize_t usbtmc_read(struct file *filp, char __user *buf,
(buffer[6] << 16) +
(buffer[7] << 24);
+ file_data->bmTransferAttributes = buffer[8];
+
if (n_characters > this_part) {
dev_err(dev, "Device wants to return more data than requested: %u > %zu\n", n_characters, count);
if (data->auto_abort)
@@ -2214,6 +2217,11 @@ static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
(__u32 __user *)arg);
break;
+ case USBTMC_IOCTL_MSG_IN_ATTR:
+ retval = put_user(file_data->bmTransferAttributes,
+ (__u8 __user *)arg);
+ break;
+
case USBTMC_IOCTL_CANCEL_IO:
retval = usbtmc_ioctl_cancel_io(file_data);
break;
diff --git a/include/uapi/linux/usb/tmc.h b/include/uapi/linux/usb/tmc.h
index 56c76a9680e5..d2ed6a20de6b 100644
--- a/include/uapi/linux/usb/tmc.h
+++ b/include/uapi/linux/usb/tmc.h
@@ -98,6 +98,8 @@ struct usbtmc_message {
#define USBTMC488_IOCTL_TRIGGER _IO(USBTMC_IOC_NR, 22)
#define USBTMC488_IOCTL_WAIT_SRQ _IOW(USBTMC_IOC_NR, 23, __u32)
+#define USBTMC_IOCTL_MSG_IN_ATTR _IOR(USBTMC_IOC_NR, 24, __u8)
+
/* Cancel and cleanup asynchronous calls */
#define USBTMC_IOCTL_CANCEL_IO _IO(USBTMC_IOC_NR, 35)
#define USBTMC_IOCTL_CLEANUP_IO _IO(USBTMC_IOC_NR, 36)
^ permalink raw reply related
* [v3,09/23] usb: usbtmc: Add ioctl USBTMC488_IOCTL_WAIT_SRQ
From: Guido Kiener @ 2018-07-24 9:05 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
Wait until an SRQ (service request) is received on the interrupt pipe
or until the given period of time is expired. In contrast to the
poll() function this ioctl does not return when other (a)synchronous
I/O operations fail with EPOLLERR.
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
---
drivers/usb/class/usbtmc.c | 57 ++++++++++++++++++++++++++++++++++++
include/uapi/linux/usb/tmc.h | 1 +
2 files changed, 58 insertions(+)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 3babca045c02..edce0a753623 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -130,6 +130,7 @@ struct usbtmc_file_data {
u32 timeout;
u8 srq_byte;
atomic_t srq_asserted;
+ atomic_t closing;
u8 eom_val;
u8 term_char;
@@ -208,6 +209,8 @@ static int usbtmc_open(struct inode *inode, struct file *filp)
mutex_lock(&data->io_mutex);
file_data->data = data;
+ atomic_set(&file_data->closing, 0);
+
/* copy default values from device settings */
file_data->timeout = USBTMC_TIMEOUT;
file_data->term_char = data->TermChar;
@@ -238,6 +241,7 @@ static int usbtmc_flush(struct file *file, fl_owner_t id)
if (file_data == NULL)
return -ENODEV;
+ atomic_set(&file_data->closing, 1);
data = file_data->data;
/* wait for io to stop */
@@ -591,6 +595,54 @@ static int usbtmc488_ioctl_read_stb(struct usbtmc_file_data *file_data,
return rv;
}
+static int usbtmc488_ioctl_wait_srq(struct usbtmc_file_data *file_data,
+ __u32 __user *arg)
+{
+ struct usbtmc_device_data *data = file_data->data;
+ struct device *dev = &data->intf->dev;
+ int rv;
+ u32 timeout;
+ unsigned long expire;
+
+ if (!data->iin_ep_present) {
+ dev_dbg(dev, "no interrupt endpoint present\n");
+ return -EFAULT;
+ }
+
+ if (get_user(timeout, arg))
+ return -EFAULT;
+
+ expire = msecs_to_jiffies(timeout);
+
+ mutex_unlock(&data->io_mutex);
+
+ rv = wait_event_interruptible_timeout(
+ data->waitq,
+ atomic_read(&file_data->srq_asserted) != 0 ||
+ atomic_read(&file_data->closing),
+ expire);
+
+ mutex_lock(&data->io_mutex);
+
+ /* Note! disconnect or close could be called in the meantime */
+ if (atomic_read(&file_data->closing) || data->zombie)
+ rv = -ENODEV;
+
+ if (rv < 0) {
+ /* dev can be invalid now! */
+ pr_debug("%s - wait interrupted %d\n", __func__, rv);
+ return rv;
+ }
+
+ if (rv == 0) {
+ dev_dbg(dev, "%s - wait timed out\n", __func__);
+ return -ETIMEDOUT;
+ }
+
+ dev_dbg(dev, "%s - srq asserted\n", __func__);
+ return 0;
+}
+
static int usbtmc488_ioctl_simple(struct usbtmc_device_data *data,
void __user *arg, unsigned int cmd)
{
@@ -2157,6 +2209,11 @@ static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
retval = usbtmc488_ioctl_trigger(file_data);
break;
+ case USBTMC488_IOCTL_WAIT_SRQ:
+ retval = usbtmc488_ioctl_wait_srq(file_data,
+ (__u32 __user *)arg);
+ break;
+
case USBTMC_IOCTL_CANCEL_IO:
retval = usbtmc_ioctl_cancel_io(file_data);
break;
diff --git a/include/uapi/linux/usb/tmc.h b/include/uapi/linux/usb/tmc.h
index d638c198c7a8..56c76a9680e5 100644
--- a/include/uapi/linux/usb/tmc.h
+++ b/include/uapi/linux/usb/tmc.h
@@ -96,6 +96,7 @@ struct usbtmc_message {
#define USBTMC488_IOCTL_GOTO_LOCAL _IO(USBTMC_IOC_NR, 20)
#define USBTMC488_IOCTL_LOCAL_LOCKOUT _IO(USBTMC_IOC_NR, 21)
#define USBTMC488_IOCTL_TRIGGER _IO(USBTMC_IOC_NR, 22)
+#define USBTMC488_IOCTL_WAIT_SRQ _IOW(USBTMC_IOC_NR, 23, __u32)
/* Cancel and cleanup asynchronous calls */
#define USBTMC_IOCTL_CANCEL_IO _IO(USBTMC_IOC_NR, 35)
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.