Re: [PATCH v2 1/6] staging/android: remove doc from sw_sync
On Mon 2016-08-08 18:24:17, Gustavo Padovan wrote: > From: Gustavo Padovan > > SW_SYNC should never be used by other pieces of the kernel apart from > sync_debug as it is only a Sync File Validation Framework, so hide any > info to avoid confuse this with a standard kernel internal API. > Signed-off-by: Gustavo Padovan NAK. It is unclear for what the code does, removing the docs is not going to help. If it should not be used, document that it should not be used.. but not remove the docs. > -/** > - * sync_timeline_signal() - signal a status change on a sync_timeline > - * @obj: sync_timeline to signal > - * @inc: num to increment on timeline->value > - * > - * A sync implementation should call this any time one of it's fences > - * has signaled or has an error condition. > - */ > static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc) > { And as the functions are static... there's little danger that someone will misuse them. Pavel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 3/4] Drivers: hv: balloon: get rid on ol_waitevent
> -Original Message- > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > Sent: Friday, August 5, 2016 3:49 AM > To: de...@linuxdriverproject.org > Cc: linux-ker...@vger.kernel.org; Haiyang Zhang ; KY > Srinivasan ; Alex Ng (LIS) > Subject: [PATCH 3/4] Drivers: hv: balloon: get rid on ol_waitevent > > With the recently introduced in-kernel memory onlining > (MEMORY_HOTPLUG_DEFAULT_ONLINE) these it no point in waiting for pages > to come online in the driver and in case the feature is disabled the 5 > second wait won't help. Get rid of the waiting. > Continuing our internal discussion here. Here's the context. > > Is it necessary to remove the ol_waitevent in "Drivers: hv: balloon: get rid > > on ol_waitevent"? If we respond to the host too quickly, then the next > > hot-add request may not see the new pages come online and could fail to > > alloc memory as seen in the call trace. > > > > Thoughts? > > This should not be an issue with CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE: we > online pages when we add them (add_memory()) so when we reply to the host > these pages are already online. But in case the onlining is done by an > external tool (e.g. udev) this wait helps (not always, as if someone eats > all memory before the next add_memory call we're still in trouble). MEMORY_HOTPLUG_DEFAULT_ONLINE is disabled in Kconfig by default. Would it make sense to keep the wait and only #ifdef it out when CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE is set? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 0/2] Drivers: hv: vmbus: make bus ids in sysfs persistent
> -Original Message- > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > Sent: Tuesday, August 9, 2016 1:46 AM > To: de...@linuxdriverproject.org > Cc: linux-ker...@vger.kernel.org; Haiyang Zhang ; > KY Srinivasan > Subject: [PATCH 0/2] Drivers: hv: vmbus: make bus ids in sysfs persistent > > Bus ids for VMBus devices in /sys/bus/vmbus/devices/ are not guaranteed > to be persistent across reboot or kernel restart and this causes problems > for some tools. E.g. kexec tools use these ids to identify NIC on kdump. > Fix the issue by using relid from channel offer as the unique id instead > of an auto incremented counter. Relids are not persistent. It is only valid between a channel offer message and a relid released message (or an unload or initiate contact message, which invalidates all channels). This is an opaque number that the root generates and uses to track channels. There is no guarantee that the same type of channel (networking, storage, etc) will get the same relid on each reboot. Regards, K. Y > > Vitaly Kuznetsov (2): > Drivers: hv: make VMBus bus ids persistent > Drivers: hv: get rid of id in struct vmbus_channel > > drivers/hv/channel_mgmt.c | 2 -- > drivers/hv/vmbus_drv.c| 2 +- > include/linux/hyperv.h| 3 --- > 3 files changed, 1 insertion(+), 6 deletions(-) > > -- > 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: sw_sync: checkpatch fixes
This patch fixes the checkpatch.pl check: CHECK: Alignment should match open parenthesis Signed-off-by: Johanna Abrahamsson --- drivers/staging/android/sw_sync.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index 115c917..ce3f3ec 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -134,7 +134,7 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc) * NULL in case of error. */ static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size, -unsigned int value) + unsigned int value) { unsigned long flags; struct sync_pt *pt; @@ -204,13 +204,13 @@ static bool timeline_fence_enable_signaling(struct fence *fence) } static void timeline_fence_value_str(struct fence *fence, - char *str, int size) +char *str, int size) { snprintf(str, size, "%d", fence->seqno); } static void timeline_fence_timeline_value_str(struct fence *fence, -char *str, int size) + char *str, int size) { struct sync_timeline *parent = fence_parent(fence); -- 2.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: ks7010: Replace three printk() calls by pr_err()
>> Please and and use pr_fmt > > Can't we use dev_* on the SDIO device? How should a connection be constructed from the data structure "sdio_device_id" to the corresponding device information for such an use case? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/7] de-stage SW_SYNC validation frawework
On Tue 2016-08-09 08:04:54, Daniel Vetter wrote: > On Sun, Jul 24, 2016 at 05:00:31PM +0200, Pavel Machek wrote: > > On Mon 2016-08-08 16:08:12, Gustavo Padovan wrote: > > > 2016-08-07 Pavel Machek : > > > > > > > On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote: > > > > > On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote: > > > > > > Hi, > > > > > > > > > > > > Do you think there is time to get this in for 4.8? > > > > > > > > > > No, it was too late on my end, due to travel and vacation, sorry. > > > > > I'll > > > > > queue it up for 4.9-rc1. > > > > > > > > Could we get some documentation what this does? Is it visilble to > > > > userspace? > > > > > > This interface is only intended for testing and validation, there are > > > ioctls on the debugfs file that can be accessed by userspace but there > > > isn't any exported kernel header with this info. The tester should know > > > and add a internal header to be able to access it. We want to prevent > > > people from misusing this feature by not advertising it nor providing > > > documentation. > > > > You are playing dangerous game here. debugfs is not normally considered > > stable, > > but otoh... ioctls on debugfs? > > It's not considered stable. The idea is that we also add the existing > testcases to kselftest. It's purely a bit of interface to be able to drive > run the test logic for real fences. What it really tests is the fence > interface (which is public in the uapi headers and all that), but to be > able to do that we need some (hw-independent way) to expose fences, which > this provides. > > Long term we might even do this as a proper interface (with some > restrictions to make it safe and avoid userspace pulling the kernel over > the table). And then rip out sw_sync entirely. > > Imo there's no need at all for docs for this. There's full directory of files, with absolutely zero comments/documentation. They are quite hard to understand. Plus it has userland interface. IMO comment should be added, explaining what it is testing, that interface is not considered stable, and where the test lives. I know what "fence" is in the cpu sense (mfence and friends), but I'm not sure what "real fence" is in this context. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 0/7] lib: string: add functions to case-convert strings
This series introduces a family of generic string case conversion functions. This kind of functionality is needed in several places in the kernel. Right now, everybody seems to be implementing their own copy of this functionality. Based on the discussion of the previous version of this series[1] and the use cases found in the kernel, it does look like having several flavours of case conversion functions is beneficial. The use cases fall into three categories: - copying a string and converting the case while specifying a maximum length to mimic strlcpy() - copying a string and converting the case without specifying a length to mimic strcpy() - converting the case of a string in-place (i.e. modifying the string that was passed in) Consequently, I am proposing these new functions: int strlcpytoupper(char *dst, const char *src, size_t len); int strlcpytolower(char *dst, const char *src, size_t len); void strcpytoupper(char *dst, const char *src); void strcpytolower(char *dst, const char *src); void strtoupper(char *s); void strtolower(char *s); Several drivers are being modified to make use of the functions above. Another driver that also makes use of this functionality will be submitted upstream shortly, which prompted this whole exercise. The changes made here have been compile-tested, but not tried out, due to lack of required hardware. Changes since v4: - rebased on 4.8-rc1; this only affects patch 6/7 (gk104) and only the patch context, not the patch itself - added an ACK to patch 4/7 (speakup) - added an ACK to patch 7/7 (power_supply) Changes since v3: - strlcpytoupper() and strlcpytolower() return length of destination or -E2BIG (see [2]) - we use ~(size_t)0 instead of -1 to copy strings of arbitrary length in strcpyto*() and strto*() - A few ACKs added Changes since v2: - use strlcpy() semantics not strncpy() semantics, i.e. guarantee NULL termination - as a result strncpyto are now called strlcpyto - make functions void - use len == -1 (SIZE_MAX) as no-limit indicator rather then len == 0 - change PATCH 2/7 to match strlcpy() semantics - change PATCH 4/7 to match strlcpy() semantics Changes since v1: - expanded strtolower() into a family of functions that cover use cases when a length argument is or isn't required and that support copying the string into a new buffer or changing it in-place - changed the function semantics to return a pointer to the terminating '\0' character of the modified string - added strtoupper() functionality mirroring the above - dropped the ACPICA patch, since that code is OS independent and can't rely on a Linux library function (see [3]) - Added two new patches replacing strtoupper() implementations [1] https://lkml.org/lkml/2016/6/30/727 [2] https://lkml.org/lkml/2016/7/10/4 [3] https://lkml.org/lkml/2016/7/1/9 Markus Mayer (7): lib: string: add functions to case-convert strings drm/nouveau/core: make use of new strlcpytolower() function ACPI / device_sysfs: make use of new strtolower() function staging: speakup: replace spk_strlwr() with strlcpytolower() iscsi-target: replace iscsi_initiatorname_tolower() with strtolower() drm/nouveau/fifo/gk104: make use of new strcpytoupper() function power_supply: make use of new strcpytoupper() function drivers/acpi/device_sysfs.c | 4 +-- drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 9 + drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c | 5 +-- drivers/power/power_supply_sysfs.c | 13 +++ drivers/staging/speakup/kobjects.c | 3 +- drivers/staging/speakup/main.c | 3 +- drivers/staging/speakup/speakup.h| 1 - drivers/staging/speakup/varhandlers.c| 12 --- drivers/target/iscsi/iscsi_target_nego.c | 17 + include/linux/string.h | 40 + lib/string.c | 46 11 files changed, 98 insertions(+), 55 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 1/7] lib: string: add functions to case-convert strings
Add a collection of generic functions to convert strings to lowercase or uppercase. Changing the case of a string (with or without copying it first) seems to be a recurring requirement in the kernel that is currently being solved by several duplicated implementations doing the same thing. This change aims at reducing this code duplication. The new functions are int strlcpytoupper(char *dst, const char *src, size_t len); int strlcpytolower(char *dst, const char *src, size_t len); void strcpytoupper(char *dst, const char *src); void strcpytolower(char *dst, const char *src); void strtoupper(char *s); void strtolower(char *s); The "str[l]cpyto*" versions of the function take a destination string and a source string as arguments. The "strlcpyto*" versions additionally take a length argument like strlcpy() itself. Lastly, the strto* functions take a single string argument and modify the passed-in string. strlcpytoupper() and strlcpytolower() return the number of characters copied or -E2BIG if the destination string was truncated. Like strlcpy(), and unlike strncpy(), the functions guarantee NULL termination of the destination string. Signed-off-by: Markus Mayer --- include/linux/string.h | 40 lib/string.c | 46 ++ 2 files changed, 86 insertions(+) diff --git a/include/linux/string.h b/include/linux/string.h index 26b6f6a..257d797 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -116,6 +116,8 @@ extern void * memchr(const void *,int,__kernel_size_t); #endif void *memchr_inv(const void *s, int c, size_t n); char *strreplace(char *s, char old, char new); +extern int strlcpytoupper(char *dst, const char *src, size_t len); +extern int strlcpytolower(char *dst, const char *src, size_t len); extern void kfree_const(const void *x); @@ -169,4 +171,42 @@ static inline const char *kbasename(const char *path) return tail ? tail + 1 : path; } +/** + * strcpytoupper - Copy string and convert to uppercase. + * @dst: The buffer to store the result. + * @src: The string to convert to uppercase. + */ +static inline void strcpytoupper(char *dst, const char *src) +{ + strlcpytoupper(dst, src, ~(size_t)0); +} + +/** + * strcpytolower - Copy string and convert to lowercase. + * @dst: The buffer to store the result. + * @src: The string to convert to lowercase. + */ +static inline void strcpytolower(char *dst, const char *src) +{ + strlcpytolower(dst, src, ~(size_t)0); +} + +/** + * strtoupper - Convert string to uppercase. + * @s: The string to operate on. + */ +static inline void strtoupper(char *s) +{ + strlcpytoupper(s, s, ~(size_t)0); +} + +/** + * strtolower - Convert string to lowercase. + * @s: The string to operate on. + */ +static inline void strtolower(char *s) +{ + strlcpytolower(s, s, ~(size_t)0); +} + #endif /* _LINUX_STRING_H_ */ diff --git a/lib/string.c b/lib/string.c index ed83562..d36d5fb2 100644 --- a/lib/string.c +++ b/lib/string.c @@ -952,3 +952,49 @@ char *strreplace(char *s, char old, char new) return s; } EXPORT_SYMBOL(strreplace); + +/** + * strlcpytoupper - Copy a length-limited string and convert to uppercase. + * @dst: The buffer to store the result. + * @src: The string to convert to uppercase. + * @len: Maximum string length. May be SIZE_MAX to set no limit. + * + * Returns the number of characters copied or -E2BIG if @dst wasn't big enough. + */ +int strlcpytoupper(char *dst, const char *src, size_t len) +{ + size_t i; + + if (!len) + return -E2BIG; + + for (i = 0; i < len && src[i]; ++i) + dst[i] = toupper(src[i]); + dst[i < len ? i : i - 1] = '\0'; + + return (i < len) ? i : -E2BIG; +} +EXPORT_SYMBOL(strlcpytoupper); + +/** + * strlcpytolower - Copy a length-limited string and convert to lowercase. + * @dst: The buffer to store the result. + * @src: The string to convert to lowercase. + * @len: Maximum string length. May be SIZE_MAX to set no limit. + * + * Returns the number of characters copied or -E2BIG if @dst wasn't big enough. + */ +int strlcpytolower(char *dst, const char *src, size_t len) +{ + size_t i; + + if (!len) + return -E2BIG; + + for (i = 0; i < len && src[i]; ++i) + dst[i] = tolower(src[i]); + dst[i < len ? i : i - 1] = '\0'; + + return (i < len) ? i : -E2BIG; +} +EXPORT_SYMBOL(strlcpytolower); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 4/7] staging: speakup: replace spk_strlwr() with strlcpytolower()
After introducing generic strltolower() and strtolower(), spk_strlwr() is no longer needed. Signed-off-by: Markus Mayer Acked-by: Samuel Thibault Acked-by: Chris Brannon --- drivers/staging/speakup/kobjects.c| 3 +-- drivers/staging/speakup/main.c| 3 ++- drivers/staging/speakup/speakup.h | 1 - drivers/staging/speakup/varhandlers.c | 12 4 files changed, 3 insertions(+), 16 deletions(-) diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c index 528cbdc..c6e0c2d 100644 --- a/drivers/staging/speakup/kobjects.c +++ b/drivers/staging/speakup/kobjects.c @@ -387,11 +387,10 @@ static ssize_t synth_store(struct kobject *kobj, struct kobj_attribute *attr, len = strlen(buf); if (len < 2 || len > 9) return -EINVAL; - strncpy(new_synth_name, buf, len); + strlcpytolower(new_synth_name, buf, sizeof(new_synth_name)); if (new_synth_name[len - 1] == '\n') len--; new_synth_name[len] = '\0'; - spk_strlwr(new_synth_name); if ((synth != NULL) && (!strcmp(new_synth_name, synth->name))) { pr_warn("%s already in use\n", new_synth_name); } else if (synth_init(new_synth_name) != 0) { diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index 97ca4ec..970f568 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -2314,7 +2314,8 @@ static int __init speakup_init(void) spk_initialize_msgs(); /* Initialize arrays for i18n. */ spk_reset_default_chars(); spk_reset_default_chartab(); - spk_strlwr(synth_name); + if (synth_name) + strtolower(synth_name); spk_vars[0].u.n.high = vc->vc_cols; for (var = spk_vars; var->var_id != MAXVARS; var++) speakup_register_var(var); diff --git a/drivers/staging/speakup/speakup.h b/drivers/staging/speakup/speakup.h index df74c91..4725785 100644 --- a/drivers/staging/speakup/speakup.h +++ b/drivers/staging/speakup/speakup.h @@ -50,7 +50,6 @@ void synth_insert_next_index(int sent_num); void spk_reset_index_count(int sc); void spk_get_index_count(int *linecount, int *sentcount); int spk_set_key_info(const u_char *key_info, u_char *k_buffer); -char *spk_strlwr(char *s); char *spk_s2uchar(char *start, char *dest); int speakup_kobj_init(void); void speakup_kobj_exit(void); diff --git a/drivers/staging/speakup/varhandlers.c b/drivers/staging/speakup/varhandlers.c index e1393d2..a1af222 100644 --- a/drivers/staging/speakup/varhandlers.c +++ b/drivers/staging/speakup/varhandlers.c @@ -309,18 +309,6 @@ int spk_set_mask_bits(const char *input, const int which, const int how) return 0; } -char *spk_strlwr(char *s) -{ - char *p; - - if (!s) - return NULL; - - for (p = s; *p; p++) - *p = tolower(*p); - return s; -} - char *spk_s2uchar(char *start, char *dest) { int val; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFCv2][PATCH 2/5] arm: Implement ARCH_HAS_FORCE_CACHE
On Mon, Aug 08, 2016 at 10:49:34AM -0700, Laura Abbott wrote: > +/* > + * Make an area consistent for devices. > + * Note: Drivers should NOT use this function directly, as it will break > + * platforms with CONFIG_DMABOUNCE. > + * Use the driver DMA support - see dma-mapping.h (dma_sync_*) > + */ > +void __dma_page_cpu_to_dev(struct page *page, unsigned long off, > + size_t size, enum dma_data_direction dir) > +{ > + phys_addr_t paddr; > + > + dma_cache_maint_page(page, off, size, dir, dmac_map_area); > + > + paddr = page_to_phys(page) + off; > + if (dir == DMA_FROM_DEVICE) { > + outer_inv_range(paddr, paddr + size); > + } else { > + outer_clean_range(paddr, paddr + size); > + } > + /* FIXME: non-speculating: flush on bidirectional mappings? */ > +} > + > +void __dma_page_dev_to_cpu(struct page *page, unsigned long off, > + size_t size, enum dma_data_direction dir) > +{ > + phys_addr_t paddr = page_to_phys(page) + off; > + > + /* FIXME: non-speculating: not required */ > + /* in any case, don't bother invalidating if DMA to device */ > + if (dir != DMA_TO_DEVICE) { > + outer_inv_range(paddr, paddr + size); > + > + dma_cache_maint_page(page, off, size, dir, dmac_unmap_area); > + } > + > + /* > + * Mark the D-cache clean for these pages to avoid extra flushing. > + */ > + if (dir != DMA_TO_DEVICE && size >= PAGE_SIZE) { > + unsigned long pfn; > + size_t left = size; > + > + pfn = page_to_pfn(page) + off / PAGE_SIZE; > + off %= PAGE_SIZE; > + if (off) { > + pfn++; > + left -= PAGE_SIZE - off; > + } > + while (left >= PAGE_SIZE) { > + page = pfn_to_page(pfn++); > + set_bit(PG_dcache_clean, &page->flags); > + left -= PAGE_SIZE; > + } > + } > +} I _really_ don't want these exposed in any shape or form to driver code. I've seen too many hacks out there where people have gone under the cover of the APIs they should be using, and headed straight for the low-level functionality - adding function prototypes to get at stuff they have no business doing. Moving this here is just asking for it to be abused. > + > +void kernel_force_cache_clean(struct page *page, size_t size) > +{ > + __dma_page_cpu_to_dev(page, 0, size, DMA_BIDIRECTIONAL); > +} > + > +void kernel_force_cache_invalidate(struct page *page, size_t size) > +{ > + __dma_page_dev_to_cpu(page, 0, size, DMA_BIDIRECTIONAL); > +} Nothing in our implementation of these DMA operations guarantees that those mean "clean" and "invalidate". The DMA operations are there so that CPUs can implement whatever they need at the map and unmap times - and I've been very careful not to specify which cache operations are involved. For example, on older CPUs, __dma_page_dev_to_cpu() is almost always a no-op. If you want something that does something specific, then we need something designed to do something specific. Please don't re-use what you think will fit. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 00/10] staging: ks7010: Fine-tuning for a SDIO card driver
From: Markus Elfring Further update suggestions were taken into account after a patch was applied from static source code analysis. Markus Elfring (10): Delete unnecessary checks before the function call "kfree" Delete unnecessary assignments for buffer variables Return directly after a failed kmalloc() Rename jump labels Delete unnecessary uses of the variable "retval" Delete unnecessary braces Add the macro "pr_fmt" Replace three printk() calls by pr_err() Delete a variable in write_to_device() Delete three unnecessary variable initialisations drivers/staging/ks7010/ks7010_sdio.c | 184 ++- 1 file changed, 73 insertions(+), 111 deletions(-) -- 2.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 01/10] staging: ks7010: Delete unnecessary checks before the function call "kfree"
From: Markus Elfring Date: Wed, 10 Aug 2016 17:15:15 +0200 The kfree() function tests whether its argument is NULL and then returns immediately. Thus the test around the calls is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/staging/ks7010/ks7010_sdio.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index b7337fd..7da6c84 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -732,8 +732,7 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index) goto error_out; } error_out: - if (data_buf) - kfree(data_buf); + kfree(data_buf); return rc; } @@ -763,8 +762,7 @@ static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address, goto error_out; } error_out: - if (read_buf) - kfree(read_buf); + kfree(read_buf); return rc; } @@ -879,8 +877,7 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv, release_firmware(fw_entry); error_out0: sdio_release_host(card->func); - if (rom_buf) - kfree(rom_buf); + kfree(rom_buf); return rc; } @@ -1199,9 +1196,7 @@ static void ks7010_sdio_remove(struct sdio_func *func) unregister_netdev(netdev); trx_device_exit(priv); - if (priv->ks_wlan_hw.read_buf) { - kfree(priv->ks_wlan_hw.read_buf); - } + kfree(priv->ks_wlan_hw.read_buf); free_netdev(priv->net_dev); card->priv = NULL; } -- 2.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 02/10] staging: ks7010: Delete unnecessary assignments for buffer variables
From: Markus Elfring Date: Wed, 10 Aug 2016 17:26:01 +0200 A few variables were assigned a null pointer despite of the detail that they were immediately reassigned by the following statement. Thus remove such unnecessary assignments. Signed-off-by: Markus Elfring --- v2: Rebased on the source files from Linux next-20160809. drivers/staging/ks7010/ks7010_sdio.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index 7da6c84..3622fba 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -711,7 +711,6 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index) int rc = 0; int retval; unsigned char *data_buf; - data_buf = NULL; data_buf = kmalloc(sizeof(u32), GFP_KERNEL); if (!data_buf) { @@ -743,7 +742,7 @@ static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address, int rc = 0; int retval; unsigned char *read_buf; - read_buf = NULL; + read_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL); if (!read_buf) { rc = 1; @@ -776,8 +775,6 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv, int length; const struct firmware *fw_entry = NULL; - rom_buf = NULL; - /* buffer allocate */ rom_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL); if (!rom_buf) { -- 2.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 03/10] staging: ks7010: Return directly after a failed kmalloc()
From: Markus Elfring Date: Wed, 10 Aug 2016 17:34:12 +0200 Return directly after a memory allocation failed at the beginning. Signed-off-by: Markus Elfring --- v2: Rebased on the source files from Linux next-20160809. drivers/staging/ks7010/ks7010_sdio.c | 19 +++ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index 3622fba..9b954cb 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -713,10 +713,8 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index) unsigned char *data_buf; data_buf = kmalloc(sizeof(u32), GFP_KERNEL); - if (!data_buf) { - rc = 1; - goto error_out; - } + if (!data_buf) + return 1; memcpy(data_buf, &index, sizeof(index)); retval = ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index)); @@ -744,10 +742,9 @@ static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address, unsigned char *read_buf; read_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL); - if (!read_buf) { - rc = 1; - goto error_out; - } + if (!read_buf) + return 1; + retval = ks7010_sdio_read(priv, address, read_buf, size); if (retval) { rc = 2; @@ -777,10 +774,8 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv, /* buffer allocate */ rom_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL); - if (!rom_buf) { - rc = 3; - goto error_out0; - } + if (!rom_buf) + return 3; sdio_claim_host(card->func); -- 2.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 04/10] staging: ks7010: Rename jump labels
From: Markus Elfring Date: Wed, 10 Aug 2016 17:57:50 +0200 Adjust jump targets according to the Linux coding style convention. Signed-off-by: Markus Elfring --- v2: Rebased on the source files from Linux next-20160809. Touched four source code places less. drivers/staging/ks7010/ks7010_sdio.c | 73 +--- 1 file changed, 34 insertions(+), 39 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index 9b954cb..53df8f7 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -523,18 +523,18 @@ static void ks7010_rw_function(struct work_struct *work) queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq, &priv->ks_wlan_hw.rw_wq, 1); } - goto err_out; + goto release_host; } /* sleep mode doze */ if (atomic_read(&priv->sleepstatus.doze_request) == 1) { ks_wlan_hw_sleep_doze_request(priv); - goto err_out; + goto release_host; } /* sleep mode wakeup */ if (atomic_read(&priv->sleepstatus.wakeup_request) == 1) { ks_wlan_hw_sleep_wakeup_request(priv); - goto err_out; + goto release_host; } /* read (WriteStatus/ReadDataSize FN1:00_0014) */ @@ -543,7 +543,7 @@ static void ks7010_rw_function(struct work_struct *work) if (retval) { DPRINTK(1, " error : WSTATUS_RSIZE=%02X psstatus=%d\n", rw_data, atomic_read(&priv->psstatus.status)); - goto err_out; + goto release_host; } DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data); @@ -555,8 +555,7 @@ static void ks7010_rw_function(struct work_struct *work) tx_device_task((void *)priv); } _ks_wlan_hw_power_save(priv); - - err_out: + release_host: sdio_release_host(priv->ks_wlan_hw.sdio_card->func); return; @@ -720,15 +719,13 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index) retval = ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index)); if (retval) { rc = 2; - goto error_out; + goto free_buf; } retval = ks7010_sdio_write(priv, READ_INDEX, data_buf, sizeof(index)); - if (retval) { + if (retval) rc = 3; - goto error_out; - } - error_out: + free_buf: kfree(data_buf); return rc; } @@ -748,16 +745,15 @@ static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address, retval = ks7010_sdio_read(priv, address, read_buf, size); if (retval) { rc = 2; - goto error_out; + goto free_buf; } retval = memcmp(data, read_buf, size); if (retval) { DPRINTK(0, "data compare error (%d) \n", retval); rc = 3; - goto error_out; } - error_out: + free_buf: kfree(read_buf); return rc; } @@ -784,7 +780,7 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv, if (rw_data == GCR_A_RUN) { DPRINTK(0, "MAC firmware running ...\n"); rc = 0; - goto error_out0; + goto release_host; } retval = request_firmware(&fw_entry, ROM_FILE, &priv->ks_wlan_hw.sdio_card->func->dev); @@ -814,14 +810,14 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv, KS7010_IRAM_ADDRESS + offset); if (retval) { rc = 6; - goto error_out1; + goto release_fw; } /* Write data */ retval = ks7010_sdio_write(priv, DATA_WINDOW, rom_buf, size); if (retval) { rc = 8; - goto error_out1; + goto release_fw; } /* compare */ @@ -829,7 +825,7 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv, ks7010_sdio_data_compare(priv, DATA_WINDOW, rom_buf, size); if (retval) { rc = 9; - goto error_out1; + goto release_fw; } n += size; @@ -840,7 +836,7 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv, retval = ks7010_sdio_write(priv, GCR_A, &rw_data, sizeof(rw_data)); if (retval) { rc = 11; - goto error_out1; + goto release_fw; } DPRINTK(4, " REMAP Request : GCR_A=%02X\n", rw_data); @@ -851,7 +847,7 @@ static int ks7010_upload_firmware(struct ks_wlan_pri
[PATCH v2 05/10] staging: ks7010: Delete unnecessary uses of the variable "retval"
From: Markus Elfring Date: Wed, 10 Aug 2016 18:56:31 +0200 A few return values can also be directly used for condition checks. Thus remove a local variable for intermediate assignments. Signed-off-by: Markus Elfring --- v2: Rebased on the source files from Linux next-20160809. Touched less source code places. drivers/staging/ks7010/ks7010_sdio.c | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index 53df8f7..0f7ea76 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -489,7 +489,6 @@ static void ks7010_rw_function(struct work_struct *work) struct hw_info_t *hw; struct ks_wlan_private *priv; unsigned char rw_data; - int retval; hw = container_of(work, struct hw_info_t, rw_wq.work); priv = container_of(hw, struct ks_wlan_private, ks_wlan_hw); @@ -538,9 +537,7 @@ static void ks7010_rw_function(struct work_struct *work) } /* read (WriteStatus/ReadDataSize FN1:00_0014) */ - retval = - ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data, sizeof(rw_data)); - if (retval) { + if (ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data, sizeof(rw_data))) { DPRINTK(1, " error : WSTATUS_RSIZE=%02X psstatus=%d\n", rw_data, atomic_read(&priv->psstatus.status)); goto release_host; @@ -708,7 +705,6 @@ static void trx_device_exit(struct ks_wlan_private *priv) static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index) { int rc = 0; - int retval; unsigned char *data_buf; data_buf = kmalloc(sizeof(u32), GFP_KERNEL); @@ -716,14 +712,12 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index) return 1; memcpy(data_buf, &index, sizeof(index)); - retval = ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index)); - if (retval) { + if (ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index))) { rc = 2; goto free_buf; } - retval = ks7010_sdio_write(priv, READ_INDEX, data_buf, sizeof(index)); - if (retval) + if (ks7010_sdio_write(priv, READ_INDEX, data_buf, sizeof(index))) rc = 3; free_buf: kfree(data_buf); -- 2.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 06/10] staging: ks7010: Delete unnecessary braces
From: Markus Elfring Date: Wed, 10 Aug 2016 19:57:01 +0200 Do not use curly brackets at some source code places where a single statement should be sufficient. Signed-off-by: Markus Elfring --- v2: Rebased on the source files from Linux next-20160809. Touched less source code places. drivers/staging/ks7010/ks7010_sdio.c | 46 +--- 1 file changed, 16 insertions(+), 30 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index 0f7ea76..2094a07 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -383,10 +383,9 @@ int ks_wlan_hw_tx(struct ks_wlan_private *priv, void *p, unsigned long size, result = enqueue_txdev(priv, p, size, complete_handler, arg1, arg2); spin_unlock(&priv->tx_dev.tx_dev_lock); - if (cnt_txqbody(priv) > 0) { + if (cnt_txqbody(priv) > 0) queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq, &priv->ks_wlan_hw.rw_wq, 0); - } return result; } @@ -402,9 +401,8 @@ static void rx_event_task(unsigned long dev) hostif_receive(priv, rp->data, rp->size); inc_rxqhead(priv); - if (cnt_rxqbody(priv) > 0) { + if (cnt_rxqbody(priv) > 0) tasklet_schedule(&priv->ks_wlan_hw.rx_bh_task); - } } return; @@ -432,9 +430,8 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size) retval = ks7010_sdio_read(priv, DATA_WINDOW, &rx_buffer->data[0], hif_align_size(size)); - if (retval) { + if (retval) goto error_out; - } /* length check */ if (size > 2046 || size == 0) { @@ -449,9 +446,8 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size) retval = ks7010_sdio_write(priv, READ_STATUS, &read_status, sizeof(read_status)); - if (retval) { + if (retval) DPRINTK(1, " error : READ_STATUS=%02X\n", read_status); - } goto error_out; } @@ -470,12 +466,11 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size) } DPRINTK(4, "READ_STATUS=%02X\n", read_status); - if (atomic_read(&priv->psstatus.confirm_wait)) { + if (atomic_read(&priv->psstatus.confirm_wait)) if (IS_HIF_CONF(event)) { DPRINTK(4, "IS_HIF_CONF true !!\n"); atomic_dec(&priv->psstatus.confirm_wait); } - } /* rx_event_task((void *)priv); */ tasklet_schedule(&priv->ks_wlan_hw.rx_bh_task); @@ -544,13 +539,11 @@ static void ks7010_rw_function(struct work_struct *work) } DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data); - if (rw_data & RSIZE_MASK) { /* Read schedule */ + if (rw_data & RSIZE_MASK) /* Read schedule */ ks_wlan_hw_rx((void *)priv, (uint16_t) (((rw_data & RSIZE_MASK) << 4))); - } - if ((rw_data & WSTATUS_MASK)) { + if (rw_data & WSTATUS_MASK) tx_device_task((void *)priv); - } _ks_wlan_hw_power_save(priv); release_host: sdio_release_host(priv->ks_wlan_hw.sdio_card->func); @@ -618,10 +611,9 @@ static void ks_sdio_interrupt(struct sdio_func *func) } DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data); rsize = rw_data & RSIZE_MASK; - if (rsize) {/* Read schedule */ + if (rsize) /* Read schedule */ ks_wlan_hw_rx((void *)priv, (uint16_t) (((rsize) << 4))); - } if (rw_data & WSTATUS_MASK) { #if 0 if (status & INT_WRITE_STATUS @@ -632,11 +624,10 @@ static void ks_sdio_interrupt(struct sdio_func *func) ks7010_sdio_write(priv, DATA_WINDOW, &rw_data, sizeof(rw_data)); - if (retval) { + if (retval) DPRINTK(1, "write DATA_WINDOW Failed!!(%d)\n", retval); - } status &= ~INT_WRITE_STATUS; } else { #endif @@ -877,13 +868,11 @@ static void ks7010_card_init(struct ks_wlan_private *priv) DPRINTK(5, "hostif_sme_enqueu()\n");
[PATCH v2 07/10] staging: ks7010: Add the macro "pr_fmt"
From: Markus Elfring Date: Wed, 10 Aug 2016 20:45:11 +0200 Add a definition for the macro "pr_fmt" so that its information can be used for consistent message output. Suggested-by: Joe Perches Signed-off-by: Markus Elfring --- v2: Addition from source code review drivers/staging/ks7010/ks7010_sdio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index 2094a07..634ef42 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -10,6 +10,8 @@ * published by the Free Software Foundation. */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include -- 2.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 08/10] staging: ks7010: Replace three printk() calls by pr_err()
From: Markus Elfring Date: Wed, 10 Aug 2016 21:56:15 +0200 Prefer usage of the macro "pr_err" over the interface "printk". Fix a typo in an error message. Signed-off-by: Markus Elfring --- v2: Rebased on the source files from Linux next-20160809. drivers/staging/ks7010/ks7010_sdio.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index 634ef42..bfca6a7 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -998,11 +998,11 @@ static int ks7010_sdio_probe(struct sdio_func *func, /* private memory allocate */ netdev = alloc_etherdev(sizeof(*priv)); if (netdev == NULL) { - printk(KERN_ERR "ks7010 : Unable to alloc new net device\n"); + pr_err(pr_fmt("Unable to alloc new net device\n")); goto release_irq; } if (dev_alloc_name(netdev, "wlan%d") < 0) { - printk(KERN_ERR "ks7010 : Couldn't get name!\n"); + pr_err(pr_fmt("Couldn't get name!\n")); goto free_dev; } @@ -1042,8 +1042,7 @@ static int ks7010_sdio_probe(struct sdio_func *func, /* Upload firmware */ ret = ks7010_upload_firmware(priv, card); /* firmware load */ if (ret) { - printk(KERN_ERR - "ks7010: firmware load failed !! retern code = %d\n", + pr_err(pr_fmt("firmware load failed! return code = %d\n"), ret); goto free_buf; } -- 2.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 09/10] staging: ks7010: Delete a variable in write_to_device()
From: Markus Elfring Date: Wed, 10 Aug 2016 22:10:20 +0200 The local variable "rc" was assigned a zero at one place. But it was not read within this function. Thus delete it. Signed-off-by: Markus Elfring Reviewed-by: Wolfram Sang --- v2: Rebased on the source files from Linux next-20160809. drivers/staging/ks7010/ks7010_sdio.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index bfca6a7..1729f57 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -299,11 +299,10 @@ static int enqueue_txdev(struct ks_wlan_private *priv, unsigned char *p, static int write_to_device(struct ks_wlan_private *priv, unsigned char *buffer, unsigned long size) { - int rc, retval; + int retval; unsigned char rw_data; struct hostif_hdr *hdr; hdr = (struct hostif_hdr *)buffer; - rc = 0; DPRINTK(4, "size=%d\n", hdr->size); if (hdr->event < HIF_DATA_REQ || HIF_REQ_MAX < hdr->event) { -- 2.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 10/10] staging: ks7010: Delete three unnecessary variable initialisations
From: Markus Elfring Date: Wed, 10 Aug 2016 22:33:52 +0200 Three variables will be set to an appropriate value a bit later. Thus omit the explicit initialisation at the beginning. Signed-off-by: Markus Elfring --- v2: Rebased on the source files from Linux next-20160809. It was omitted to reduce the scope for the variable "rc" in the function "tx_device_task". drivers/staging/ks7010/ks7010_sdio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index 1729f57..bb6294c 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -331,7 +331,7 @@ static void tx_device_task(void *dev) { struct ks_wlan_private *priv = (struct ks_wlan_private *)dev; struct tx_device_buffer *sp; - int rc = 0; + int rc; DPRINTK(4, "\n"); if (cnt_txqbody(priv) > 0 @@ -366,7 +366,7 @@ int ks_wlan_hw_tx(struct ks_wlan_private *priv, void *p, unsigned long size, void (*complete_handler) (void *arg1, void *arg2), void *arg1, void *arg2) { - int result = 0; + int result; struct hostif_hdr *hdr; hdr = (struct hostif_hdr *)p; @@ -747,7 +747,7 @@ static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address, static int ks7010_upload_firmware(struct ks_wlan_private *priv, struct ks_sdio_card *card) { - unsigned int size, offset, n = 0; + unsigned int size, offset, n; unsigned char *rom_buf; unsigned char rw_data = 0; int retval, rc = 0; -- 2.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel