Re: [GIT PULL] Staging/IIO driver patches for 4.19-rc1
On Tue, Aug 28, 2018 at 02:30:49PM +, Ahmed S. Darwish wrote:
> Hi!
>
> On Tue, Aug 28, 2018 at 02:36:07PM +0200, Greg KH wrote:
> > On Tue, Aug 28, 2018 at 10:38:17AM +, Ahmed S. Darwish wrote:
> > > [ re-send; forgotten lkml CC added; sorry ]
> > >
> > > Hi,
> > >
> > > On Sat, 18 Aug 2018 17:57:24 +0200, Greg KH wrote:
> > > [...]
> > > > addition of some new IIO drivers. Also added was a "gasket" driver from
> > > > Google that needs loads of work and the erofs filesystem.
> > > >
> > >
> > > Why are we adding __a whole new in-kernel framework__ for
> > > developing basic user-space drivers?
> > >
> > > We already have a frameowrk for that, and it's UIO. [1] The UIO
> > > code is a very stable and simple subsystem; it's also heavily used
> > > in the embedded industry..
> >
> > As Todd said, the code will end up being a simple UIO driver, if even
> > that big, in th end. It is just going to take him a while to constantly
> > refactor things until he achieves that goal...
> >
> > > I've looked at the gasket documentation [2], and the first user
> > > of this new in-kernel API [3], and this is almost replicating UIO
> > > it's not funny. [4] True, the gasket APIs adds some extra new
> > > conveniences (PCI BAR re-mapping, MSI, ..), but there's no
> > > technical reason this cannot be added to the UIO code instead.
> >
> > {shh} That's my plan :)
> >
>
> Cool, thanks a lot.
>
> Can we then merge something like below patch?
>
> [ I've searched the gasket included TODO file before posting,
> but did not find any mention of UIO. Below patch will make
> sure this will not be forgotten over time.. ]
Looks good, can you resend it in a format I can apply it in (i.e. one
that does not require me to hand-edit a patch?)
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging:rtl8192u: Rename dot11d_init to fix name clash
On Tue, Sep 04, 2018 at 11:56:23AM +0100, John Whitmore wrote: > The function dot11d_init() was previously renamed to clear a style > issue. Unfortunately the new name used, dot11d_init(), clashes with > a sybmol which is exported with the same name. To correct this > problem the function has been renamed to rtl8192u_dot11d_init(). > > Signed-off-by: John Whitmore You forgot a "Reported-by:" tag here. I'll go add it... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add per-heap counters
On Sun, Sep 09, 2018 at 01:44:31AM +0300, Alexey Skidanov wrote:
> The heap statistics have been removed and currently even basics statistics
> are missing.
>
> This patch creates per heap debugfs directory /sys/kernel/debug/
> and adds two counters - the number of allocated buffers and number of
> allocated bytes.
Why? What can we do with this information?
>
> Signed-off-by: Alexey Skidanov
> ---
> drivers/staging/android/ion/ion.c | 40
> +++
> drivers/staging/android/ion/ion.h | 3 ++-
> 2 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion.c
> b/drivers/staging/android/ion/ion.c
> index 9907332..62335b9 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -95,6 +95,9 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
> *heap,
> goto err1;
> }
>
> + heap->num_of_buffers++;
> + heap->num_of_alloc_bytes += len;
> +
> INIT_LIST_HEAD(&buffer->attachments);
> mutex_init(&buffer->lock);
> mutex_lock(&dev->buffer_lock);
> @@ -528,6 +531,8 @@ void ion_device_add_heap(struct ion_heap *heap)
> {
> struct ion_device *dev = internal_dev;
> int ret;
> + struct dentry *heap_root;
> + char debug_name[64];
>
> if (!heap->ops->allocate || !heap->ops->free)
> pr_err("%s: can not add heap with invalid ops struct.\n",
> @@ -546,6 +551,33 @@ void ion_device_add_heap(struct ion_heap *heap)
> }
>
> heap->dev = dev;
> + heap->num_of_buffers = 0;
> + heap->num_of_alloc_bytes = 0;
> +
> + /* Create heap root directory. If creation is failed, we may continue */
> + heap_root = debugfs_create_dir(heap->name, dev->debug_root);
> + if (!IS_ERR_OR_NULL(heap_root)) {
Close, but no, never check the return value here. Just keep on moving,
and call further debugfs calls. No need even for a comment here.
> + debugfs_create_u64("num_of_buffers",
> +0444, heap_root,
> +&heap->num_of_buffers);
> + debugfs_create_u64("num_of_alloc_bytes",
> +0444,
> +heap_root,
> +&heap->num_of_alloc_bytes);
> +
> + if (heap->shrinker.count_objects &&
> + heap->shrinker.scan_objects) {
> + snprintf(debug_name, 64, "%s_shrink", heap->name);
> + debugfs_create_file(debug_name,
> + 0644,
> + heap_root,
> + heap,
> + &debug_shrink_fops);
> + }
> + } else {
> + pr_err("%s: Failed to create heap dir in debugfs\n", __func__);
No need to print anything.
> + }
> +
> down_write(&dev->lock);
> heap->id = heap_id++;
> /*
> @@ -555,14 +587,6 @@ void ion_device_add_heap(struct ion_heap *heap)
> plist_node_init(&heap->node, -heap->id);
> plist_add(&heap->node, &dev->heaps);
>
> - if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
> - char debug_name[64];
> -
> - snprintf(debug_name, 64, "%s_shrink", heap->name);
> - debugfs_create_file(debug_name, 0644, dev->debug_root,
> - heap, &debug_shrink_fops);
> - }
You just moved the location of these debugfs files, right? What
userspace tool just broke when you did that? :)
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: clocking-wizard: Add support for dynamic reconfiguration
On Mon, Sep 10, 2018 at 10:47:28AM +0530, shubhrajyoti.da...@gmail.com wrote: > From: Shubhrajyoti Datta > > The patch adds support for dynamic reconfiguration of clock output rate. > Output clocks are registered as dividers and set rate callback function > is used for dynamic reconfiguration. > > Signed-off-by: Chirag Parekh > Signed-off-by: Shubhrajyoti Datta > --- > .../clocking-wizard/clk-xlnx-clock-wizard.c| 211 > - > 1 file changed, 206 insertions(+), 5 deletions(-) Why are you adding new features here and not working on getting this driver out of the staging portion of the kernel? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/12] staging: wlan-ng: avoid use of camel case macro names in p80211metadef.h
On Thu, Aug 30, 2018 at 11:28:13AM +0100, Tim Collier wrote: > On Wed, Aug 29, 2018 at 02:37:36PM +0300, Dan Carpenter wrote: > > On Tue, Aug 28, 2018 at 08:26:13PM +0100, Tim Collier wrote: > > > checkpatch reported a number of "Avoid CamelCase" issues for macros > > > defined in p80211metadef.h (and for files that used these macros). > > > > > > Renamed the macros to all upper-case (except for > > > DIDmib_dot11smt_dot11WEPDefaultKeysTable_key, a function-like macro, > > > which was renamed to all lower-case) to conform to the coding > > > guidelines. > > > > > > > We rename variables and then rename them again to shorter names. It > > feels like we should just do it one time... > > I split into two steps in case the shortening was rejected and also > because the large number of changes for this particular patch seemed > easier for review if the changes were simple and repetitive. > > Happy to change it, though. How would you recommend organizing? One > patch per name changed, one per group or just one big hit including > case change and shortenings? One patch per name changed is usually easier to review. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add per-heap counters
On Mon, Sep 10, 2018 at 01:46:13PM +0300, Alexey Skidanov wrote: > > > On 09/10/2018 11:27 AM, Greg KH wrote: > > On Sun, Sep 09, 2018 at 01:44:31AM +0300, Alexey Skidanov wrote: > >> The heap statistics have been removed and currently even basics statistics > >> are missing. > >> > >> This patch creates per heap debugfs directory /sys/kernel/debug/ > >> and adds two counters - the number of allocated buffers and number of > >> allocated bytes. > > > > Why? What can we do with this information? > Are you in doubt that we need these two specific counters or debugfs at all? These specific counters. What are you going to do with them? And you also moved debugfs files (you didn't comment on that comment), which will probably break tools that expect the files to be in one place, right? How did you test this patch? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/21] staging:rtl8192u: Rename AdvCoding - Style
On Wed, Aug 29, 2018 at 09:35:27PM +0100, John Whitmore wrote:
> Rename the bit field element AdvCoding, as it causes a checkpatch issue
> with CamelCase naming. As the element is not actually used in code it
> has been renamed to 'not_used_adv_coding'.
>
> The single line of code which initialises the bit has been removed,
> as the field is unused.
>
> This is a purely coding style change which should have no impact
> on runtime code execution.
>
> Signed-off-by: John Whitmore
> ---
> drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 +-
> drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 1 -
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> index 64d5359cf7e2..66a0274077d3 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> @@ -39,7 +39,7 @@ enum ht_extension_chan_offset {
>
> struct ht_capability_ele {
> //HT capability info
> - u8 AdvCoding:1;
> + u8 not_used_adv_coding:1;
> u8 ChlWidth:1;
> u8 MimoPwrSave:2;
> u8 GreenField:1;
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> index 9bf52cebe4cd..d7f73a5831da 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> @@ -550,7 +550,6 @@ void HTConstructCapabilityElement(struct ieee80211_device
> *ieee, u8 *posHTCap, u
> }
>
> //HT capability info
> - pCapELE->AdvCoding = 0; // This feature is not supported
> now!!
> if (ieee->GetHalfNmodeSupportByAPsHandler(ieee->dev))
> pCapELE->ChlWidth = 0;
> else
If a structure like this, which is not read/written to hardware
directly, has fields that are not really used at all, just delete them.
That's much easier than having to apply this patch, and a later patch
which deletes the usage.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 20/20] staging:rtl8192u: Rename OWN - Style
On Sat, Sep 01, 2018 at 12:02:50AM +0100, John Whitmore wrote:
> Rename the member variable 'OWN' to 'own', this is to comply with the
> coding standard, where variables are named in lowercase.
>
> This is a simple coding style change which should have no impact on
> runtime code execution.
>
> Signed-off-by: John Whitmore
> ---
> drivers/staging/rtl8192u/r8192U.h | 2 +-
> drivers/staging/rtl8192u/r8192U_core.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/r8192U.h
> b/drivers/staging/rtl8192u/r8192U.h
> index 68e82b32a88e..f963b664e221 100644
> --- a/drivers/staging/rtl8192u/r8192U.h
> +++ b/drivers/staging/rtl8192u/r8192U.h
> @@ -165,7 +165,7 @@ struct tx_desc_819x_usb {
> u8 last_seg:1;
> u8 first_seg:1;
> u8 linip:1;
> - u8 OWN:1;
> + u8 own:1;
>
> /* DWORD 1 */
> u8 TxFWInfoSize;
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c
> b/drivers/staging/rtl8192u/r8192U_core.c
> index 911d0214b48a..4d0f70ec31ac 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -1558,7 +1558,7 @@ short rtl8192_tx(struct net_device *dev, struct sk_buff
> *skb)
> /* DWORD 0 */
> tx_desc->first_seg = 1;
> tx_desc->last_seg = 1;
> - tx_desc->OWN = 1;
> + tx_desc->own = 1;
Is this another case of a variable being set once and then never used?
If so, just delete it.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 19/20] staging:rtl8192u: Rename LINIP - Style
On Sat, Sep 01, 2018 at 12:02:49AM +0100, John Whitmore wrote:
> Rename the member variable 'LINIP' to 'linip', this change is to
> conform to the coding style guidelines, member variables in
> lowercase.
>
> This is a simple coding style change which should not impact runtime
> code execution.
>
> Signed-off-by: John Whitmore
> ---
> drivers/staging/rtl8192u/r8192U.h | 2 +-
> drivers/staging/rtl8192u/r8192U_core.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/r8192U.h
> b/drivers/staging/rtl8192u/r8192U.h
> index 66efa59eabf0..68e82b32a88e 100644
> --- a/drivers/staging/rtl8192u/r8192U.h
> +++ b/drivers/staging/rtl8192u/r8192U.h
> @@ -164,7 +164,7 @@ struct tx_desc_819x_usb {
> u8 cmd_init:1;
> u8 last_seg:1;
> u8 first_seg:1;
> - u8 LINIP:1;
> + u8 linip:1;
> u8 OWN:1;
>
> /* DWORD 1 */
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c
> b/drivers/staging/rtl8192u/r8192U_core.c
> index a4d1b55a1117..911d0214b48a 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -1514,7 +1514,7 @@ short rtl8192_tx(struct net_device *dev, struct sk_buff
> *skb)
> /* Fill Tx descriptor */
> memset(tx_desc, 0, sizeof(struct tx_desc_819x_usb));
> /* DWORD 0 */
> - tx_desc->LINIP = 0;
> + tx_desc->linip = 0;
Please just delet.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 18/20] staging:rtl8192u: Rename FirstSeg - Style
On Sat, Sep 01, 2018 at 12:02:48AM +0100, John Whitmore wrote:
> Rename the member variable 'FirstSeg' to 'first_seg', this change
> clears the checkpatch issue with CamelCase naming.
>
> This is a simple coding style change and as such should not impact
> runtime code execution.
>
> Signed-off-by: John Whitmore
> ---
> drivers/staging/rtl8192u/r8192U.h | 2 +-
> drivers/staging/rtl8192u/r8192U_core.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/r8192U.h
> b/drivers/staging/rtl8192u/r8192U.h
> index 672cbfa688f7..66efa59eabf0 100644
> --- a/drivers/staging/rtl8192u/r8192U.h
> +++ b/drivers/staging/rtl8192u/r8192U.h
> @@ -163,7 +163,7 @@ struct tx_desc_819x_usb {
> u8 reserved0:3;
> u8 cmd_init:1;
> u8 last_seg:1;
> - u8 FirstSeg:1;
> + u8 first_seg:1;
> u8 LINIP:1;
> u8 OWN:1;
>
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c
> b/drivers/staging/rtl8192u/r8192U_core.c
> index dfbba1177470..a4d1b55a1117 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -1556,7 +1556,7 @@ short rtl8192_tx(struct net_device *dev, struct sk_buff
> *skb)
>* all of the descriptors
>*/
> /* DWORD 0 */
> - tx_desc->FirstSeg = 1;
> + tx_desc->first_seg = 1;
Same for all of these, I'll let you respin this patch series...
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] erofs: surround fault_injection ralted option parsing using CONFIG_EROFS_FAULT_INJECTION
On Mon, Sep 10, 2018 at 11:45:51PM +0800, Chengguang Xu wrote:
> It's a little bit strange when fault_injection related
> option fail with -EINVAL which was already disabled
> from config, so surround all fault_injection related option
> parsing code using CONFIG_EROFS_FAULT_INJECTION. Meanwhile,
> slightly change warning message to keep consistency with
> option POSIX_ACL and FS_XATTR.
>
> Signed-off-by: Chengguang Xu
> ---
> drivers/staging/erofs/super.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index 1aec509c805f..4004a00d150d 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -237,16 +237,18 @@ static int parse_options(struct super_block *sb, char
> *options)
> infoln("noacl options not supported");
> break;
> #endif
> +#ifdef CONFIG_EROFS_FAULT_INJECTION
> case Opt_fault_injection:
> if (args->from && match_int(args, &arg))
> return -EINVAL;
> -#ifdef CONFIG_EROFS_FAULT_INJECTION
> erofs_build_fault_attr(EROFS_SB(sb), arg);
> set_opt(EROFS_SB(sb), FAULT_INJECTION);
> + break;
> #else
> - infoln("FAULT_INJECTION was not selected");
> -#endif
> + case Opt_fault_injection:
> + infoln("FAULT_INJECTION not supported");
> break;
> +#endif
Ugh, that's hard to read. Why not just hide all of this crazyness
behind a single function that handles the #ifdef mess, like it should
be. Then just always call handle_fault_injection() or whatever you want
to call it.
That makes it easier to read and understand and maintain over time.
I'll take this for now, but that should be something you work on
eventually. For the other #ifdef options in here as well.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add per-heap counters
On Mon, Sep 10, 2018 at 06:51:18PM +0300, Alexey Skidanov wrote:
> On 09/10/2018 05:21 PM, Greg KH wrote:
> > On Mon, Sep 10, 2018 at 01:46:13PM +0300, Alexey Skidanov wrote:
> >> On 09/10/2018 11:27 AM, Greg KH wrote:
> > And you also moved debugfs files (you didn't comment on that comment),
> > which will probably break tools that expect the files to be in one
> > place, right?
> Right. I assumed that for staging modules we still can change user
> interface. Otherwise, how to explain this change 15c6098cfec5 ("staging:
> android: ion: Remove ion_handle and ion_client")?
Yes, we can change them, but I asked if there were existing scripts that
would break with this change, which isn't good. I know there are a
number of android debug/crash tools/scripts that dig in debugfs to get
lots of information, so breaking them is usually not nice :)
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: android: ion: Add per-heap counters
On Tue, Sep 11, 2018 at 11:50:19AM +0300, Dan Carpenter wrote:
> On Tue, Sep 11, 2018 at 11:17:10AM +0300, Alexey Skidanov wrote:
> > @@ -546,6 +556,38 @@ void ion_device_add_heap(struct ion_heap *heap)
> > }
> >
> > heap->dev = dev;
> > + heap->num_of_buffers = 0;
> > + heap->num_of_alloc_bytes = 0;
> > + heap->alloc_bytes_wm = 0;
> > +
> > + /* Create heap root directory. If creation is failed, we may continue */
> > + heap_root = debugfs_create_dir(heap->name, dev->debug_root);
> > + if (!IS_ERR_OR_NULL(heap_root)) {
>
> Just remove this check. If debugfs_create_dir() fails, it doesn't
> cause any problems here.
I asked for that to be done last time :(
{sigh}
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: android: ion: Add per-heap counters
On Tue, Sep 11, 2018 at 12:11:23PM +0300, Alexey Skidanov wrote:
>
>
> On 09/11/2018 11:59 AM, Greg KH wrote:
> > On Tue, Sep 11, 2018 at 11:50:19AM +0300, Dan Carpenter wrote:
> >> On Tue, Sep 11, 2018 at 11:17:10AM +0300, Alexey Skidanov wrote:
> >>> @@ -546,6 +556,38 @@ void ion_device_add_heap(struct ion_heap *heap)
> >>> }
> >>>
> >>> heap->dev = dev;
> >>> + heap->num_of_buffers = 0;
> >>> + heap->num_of_alloc_bytes = 0;
> >>> + heap->alloc_bytes_wm = 0;
> >>> +
> >>> + /* Create heap root directory. If creation is failed, we may continue */
> >>> + heap_root = debugfs_create_dir(heap->name, dev->debug_root);
> >>> + if (!IS_ERR_OR_NULL(heap_root)) {
> >>
> >> Just remove this check. If debugfs_create_dir() fails, it doesn't
> >> cause any problems here.
> If debugfs_create_dir fails, the heap_root will be either NULL (and thus
> the underlying files will be created under /sys/kernel/debug) or -EXXX
> (and thus we probably crash the Kernel). Am I wrong?
If the result is NULL, then something bad is wrong with debugfs, so any
future calls will also fail, no need to worry.
If -ENODEV returns, then you can pass that into a future debugfs call
just fine as well, all is good.
> > I asked for that to be done last time :(
> What exactly?
I asked that you not check the return value of the debugfs call, it's
explicitly designed to not need to do this. Don't do extra work for no
reason :)
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: KASAN: null-ptr-deref Write in binder_update_page_range
On Fri, Sep 07, 2018 at 12:34:19PM +0900, Minchan Kim wrote: > Thanks, Martijn, > > Greg, could you have a look to pick up? Now queued up, thanks. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[GIT PULL] Staging/IIO driver fixes for 4.19-rc4
The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3: Linux 4.19-rc1 (2018-08-26 14:11:59 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git tags/staging-4.19-rc4 for you to fetch changes up to 65aac17423284634169489f298169c3e3f099cc7: staging: vboxvideo: Change address of scanout buffer on page-flip (2018-09-11 18:39:54 +0200) Staging/IIO fixes for 4.19-rc4 Here are a few small staging and iio driver fixes for -rc4. Nothing major, just a few small bugfixes for some reported issues, and a MAINTAINERS file update for the fbtft drivers. We also re-enable the building of the erofs filesystem as the patcheset that was causing it to break never got merged in the -rc1 cycle, so there's no reason it can't be turned back on for now. The problem that was previously there is now being handled in that other tree at the moment, so it will not hit us again in the future. All of these patches have been in linux-next with no reported issues. Signed-off-by: Greg Kroah-Hartman Ahmed S. Darwish (1): staging: gasket: TODO: re-implement using UIO Arnd Bergmann (1): staging: wilc1000: revert "fix TODO to compile spi and sdio components in single module" Daniel Vetter (1): staging/fbtft: Update TODO and mailing lists Gao Xiang (2): Revert "staging: erofs: disable compiling temporarile" staging: erofs: rename superblock flags (MS_xyz -> SB_xyz) Greg Kroah-Hartman (1): Merge tag 'iio-fixes-4.19a' of git://git.kernel.org/.../jic23/iio into staging-linus Hans de Goede (2): staging: vboxvideo: Fix IRQs no longer working staging: vboxvideo: Change address of scanout buffer on page-flip Lorenzo Bianconi (1): iio: imu: st_lsm6dsx: take into account ts samples in wm configuration Matt Ranostay (1): Revert "iio: temperature: maxim_thermocouple: add MAX31856 part" Todd Poynor (1): MAINTAINERS: Switch a maintainer for drivers/staging/gasket MAINTAINERS| 4 +++- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 13 ++--- drivers/iio/temperature/maxim_thermocouple.c | 1 - drivers/staging/erofs/Kconfig | 2 +- drivers/staging/erofs/super.c | 4 ++-- drivers/staging/fbtft/TODO | 4 drivers/staging/gasket/TODO| 13 + drivers/staging/vboxvideo/vbox_drv.c | 7 +++ drivers/staging/vboxvideo/vbox_mode.c | 5 + drivers/staging/wilc1000/Makefile | 3 +-- drivers/staging/wilc1000/linux_wlan.c | 6 -- drivers/staging/wilc1000/wilc_debugfs.c| 7 +-- drivers/staging/wilc1000/wilc_wlan.c | 6 ++ drivers/staging/wilc1000/wilc_wlan_if.h| 2 -- 14 files changed, 57 insertions(+), 20 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown operation was requested
On Mon, Sep 17, 2018 at 04:14:55AM +, k...@linuxonhyperv.com wrote: > From: Vitaly Kuznetsov > > 'error' variable is left uninitialized in case we see an unknown operation. > As we don't immediately return and proceed to pwrite() we need to set it > to something, HV_E_FAIL sounds good enough. > > Signed-off-by: Vitaly Kuznetsov > Signed-off-by: K. Y. Srinivasan > --- > tools/hv/hv_fcopy_daemon.c | 1 + > 1 file changed, 1 insertion(+) No need to backport for stable? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown operation was requested
On Mon, Sep 17, 2018 at 02:16:48PM +, KY Srinivasan wrote: > > > > -Original Message- > > From: Greg KH > > Sent: Sunday, September 16, 2018 9:56 PM > > To: KY Srinivasan > > Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; > > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > > Hemminger ; Michael Kelley (EOSG) > > ; vkuznets > > Subject: Re: [PATCH 2/2] tools: hv: fcopy: set 'error' in case an unknown > > operation was requested > > > > On Mon, Sep 17, 2018 at 04:14:55AM +, k...@linuxonhyperv.com wrote: > > > From: Vitaly Kuznetsov > > > > > > 'error' variable is left uninitialized in case we see an unknown > > > operation. > > > As we don't immediately return and proceed to pwrite() we need to set it > > > to something, HV_E_FAIL sounds good enough. > > > > > > Signed-off-by: Vitaly Kuznetsov > > > Signed-off-by: K. Y. Srinivasan > > > --- > > > tools/hv/hv_fcopy_daemon.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > No need to backport for stable? > Thanks for pointing this out. Should I resubmit with the stable tag? I can do it... :) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: android: ion: Add per-heap counters
On Tue, Sep 11, 2018 at 02:29:19PM +0300, Alexey Skidanov wrote:
> Heap statistics have been removed and currently even basics statistics
> are missing.
>
> This patch creates per heap debugfs directory /sys/kernel/debug/
> and adds the following counters:
> - the number of allocated buffers;
> - the number of allocated bytes;
> - the number of allocated bytes watermark.
>
> Signed-off-by: Alexey Skidanov
> ---
It would be great to get an ack from some of the ion developers...
{hint}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: leaking path in android binder: set_nice
On Tue, Sep 25, 2018 at 01:27:11PM -0400, Tong Zhang wrote: > Kernel Version: 4.18.5 > > Problem Description: > > When setting nice value, it is checked by LSM function > security_task_setnice(). > see kernel/sched/core.c:3972 SYSCALL_DEFINE1(nice, int, increment) > > We discovered a leaking path in android binder which allows using binder’s > interface to change > a process’s nice value. This path is leaked from being monitored by LSM. > see drivers/android/binder.c:1107 binder_set_nice. Can you please submit a patch for this to get the proper credit for finding and fixing this? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused
On Wed, Sep 26, 2018 at 11:28:46AM -0700, Nick Desaulniers wrote:
> On Wed, Sep 26, 2018 at 10:55 AM Nick Desaulniers
> wrote:
> >
> > On Wed, Sep 26, 2018 at 12:41 AM Nathan Chancellor
> > wrote:
> > >
> > > On Wed, Sep 26, 2018 at 09:13:59AM +0200, Greg Kroah-Hartman wrote:
> > > > On Wed, Sep 26, 2018 at 12:02:09AM -0700, Nathan Chancellor wrote:
> > > > > Clang emits the following warning:
> > > > >
> > > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable
> > > > > 'acpi_ids' is not needed and will not be emitted
> > > > > [-Wunneeded-internal-declaration]
> > > > > static const struct acpi_device_id acpi_ids[] = {
> > > > >^
> > > > > 1 warning generated.
> > > > >
> > > > > Mark the declaration as maybe unused like a few other instances of
> > > > > this
> > > > > construct in the kernel.
> > > > >
> > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/169
> > > > > Signed-off-by: Nathan Chancellor
> > > > > ---
> > > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > > b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > > index 6d02904de63f..3285bf36291b 100644
> > > > > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > > @@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] =
> > > > > { SDIO_DEVICE(0x024c, 0xb723), },
> > > > > { /* end: all zeroes */ },
> > > > > };
> > > > > -static const struct acpi_device_id acpi_ids[] = {
> > > > > +static const struct acpi_device_id acpi_ids[] __maybe_unused = {
> > > >
> > > > But it is used. No "maybe" at all here. The MODULE_DEVICE_TABLE()
> > > > macro does a functional thing. Why is gcc not reporting an issue with
> > > > this and clang is?
> >
> > Looks like GCC and Clang handle __attribute__((alias)) differently
> > when optimizations are enabled. Clang has
> > -Wunneeded-internal-declaration (GCC doesn't have that specific flag,
> > which is why GCC doesn't report, but its behavior is also different),
> > as part of -Wall, which warns that it thinks the static var is dead,
> > then removes it from -O1 and up.
> >
> > https://godbolt.org/z/Dpxnbp
> >
> > I consider this a bug in Clang: https://bugs.llvm.org/show_bug.cgi?id=39088
> >
> > As Nathan notes in this comment in V1:
> > https://lore.kernel.org/lkml/20180926064437.GA29417@flashbox/ having
> > additional references to the static array is enough to keep it around.
> > When there are no other references, then it should not be marked
> > static, in order to still be emitted.
> >
> > We can work around this by removing `static` from struct definitions
> > that no other references than being passed to the MODULE_DEVICE_TABLE
> > macro. GCC and Clang will then both emit references to both
> > identifiers.
>
> Sorry, after thinking more about this and discussing more with a
> coworker, I think Clang is doing the right thing, and that `static`
> should be removed from declarations passed to MODULE_DEVICE_TABLE
> without other references. Clang's Dead Code Elimination (DCE) here
> seems to be more aggressive, but it still looks correct to me.
>
> int foo [] = { 42, 0xDEAD }; // extern
> extern typeof(foo) bar __attribute__((unused, alias("foo")));
>
> GCC and Clang at -O2 both produce references to foo and bar.
>
> Adding `static` to foo, then GCC produces both references, while Clang
> moves the data to bar and removes foo. This is safe because foo has
> no other references, and bar is what file2alias.c cares about in this
> case.
But we want the variable to be static, and not part of any namespace
outside of this file. If we start changing them all, we will start
running into namespace collisions.
These go into a separate part of the linker table, shouldn't that be
enough to ensure that the compiler keeps it around? Or is that after
the compiler processes things?
Anything we can do to add to the MODULE_DEVICE_TABLE() macro instead?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 06/13] staging:rtl8192u: Remove TxSTBC and RxSTBC - Style
On Wed, Sep 26, 2018 at 08:16:57PM +0100, John Whitmore wrote: > Remove the member variables TxSTBC and RxSTBC as neither is used in > code. > > This is a coding style change which should not impact runtime code > execution. Same as before, I think this _does_ impact runtime :( I'll stop here in the series. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style
On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> The member variables AdvCoding and GreenField are unused in code so
> have been removed from the structure and associated initialisation
> function.
>
> This is a coding style change which should have no impact on runtime
> code execution.
>
> Signed-off-by: John Whitmore
> ---
> drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
> drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
> 2 files changed, 4 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> index 64d5359cf7e2..83fb8f34ccbd 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
>
> struct ht_capability_ele {
> //HT capability info
> - u8 AdvCoding:1;
> u8 ChlWidth:1;
> u8 MimoPwrSave:2;
> - u8 GreenField:1;
Don't these fields come from the hardware itself? By removing them
here, you just changed the memory layout of the structure. Does the
driver still work properly after this? If you can't test it, I can't
take this patch as it's too risky...
sorry,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtlwifi: coding style fixes
On Sat, Sep 29, 2018 at 11:47:47AM +0800, spring full wrote:
> According to Documentation/process/coding-style.rst,
> ``13) Printing kernel messages ..
> Do not use crippled words like ``dont``; use ``do not`` or ``don't``
> instead``,
> the word dont is changed to don't.
>
> Signed-off-by: Chunguang Wu
This name did not match the name in the From: line of your email :(
> ---
> drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
> b/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
> index 4d1f9bf..d06a747 100644
> --- a/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
> +++ b/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
> @@ -292,7 +292,7 @@ static void halbtc_leave_lps(struct btc_coexist
> *btcoexist)
>
> if (ap_enable) {
> RT_TRACE(rtlpriv, COMP_BT_COEXIST, DBG_DMESG,
> -"%s()<--dont leave lps under AP mode\n", __func__);
> +"%s()<--don't leave lps under AP mode\n",
> __func__);
> return;
> }
>
> @@ -315,7 +315,7 @@ static void halbtc_enter_lps(struct btc_coexist
> *btcoexist)
>
> if (ap_enable) {
> RT_TRACE(rtlpriv, COMP_BT_COEXIST, DBG_DMESG,
> -"%s()<--dont enter lps under AP mode\n", __func__);
> +"%s()<--don't enter lps under AP mode\n",
> __func__);
> return;
> }
>
You email client corrupted all of the whitespace, so I could not apply
this even if I wanted to :(
Please fix up and resend.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: rts5208: rtsx_card: Fixed multiple coding style issues
On Fri, Sep 28, 2018 at 10:03:00PM -0400, Maxime Desroches wrote: > Fixed multiple coding style issues > > Signed-off-by: Maxime Desroches > --- > drivers/staging/rts5208/rtsx_card.c | 96 +++-- > 1 file changed, 37 insertions(+), 59 deletions(-) Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - Your patch did many different things all at once, making it difficult to review. All Linux kernel patches need to only do one thing at a time. If you need to do multiple things (such as clean up all coding style issues in a file/driver), do it in a sequence of patches, each one doing only one thing. This will make it easier to review the patches to ensure that they are correct, and to help alleviate any merge issues that larger patches can cause. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] Staging: rts5208: xd.c: Fixed all braces issues of the file
On Sat, Sep 29, 2018 at 02:00:27PM -0400, Maxime Desroches wrote: > Fixed all the braces issues of the xd.c file > > Signed-off-by: Maxime Desroches > --- > drivers/staging/rts5208/xd.c | 232 ++- > 1 file changed, 92 insertions(+), 140 deletions(-) Again, missing earlier patches. Please fix this up and resend as a proper series of patches, properly numbered. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] Staging: rts5208: spi.c: Fixed all braces issues of the file
On Sat, Sep 29, 2018 at 12:09:50PM -0400, Maxime Desroches wrote: > Fixed all the braces issues of the spi.c file > > Signed-off-by: Maxime Desroches > --- > drivers/staging/rts5208/spi.c | 153 +- > 1 file changed, 59 insertions(+), 94 deletions(-) where are patches 1-4 out of this 5 patch series? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtlwifi: coding style fixes
On Sat, Sep 29, 2018 at 09:09:02PM +0800, Chunguang Wu wrote:
> According to Documentation/process/coding-style.rst,
> ``13) Printing kernel messages ..
> Do not use crippled words like ``dont``; use ``do not`` or ``don't``
> instead``,
> the word dont is changed to don't.
>
> Signed-off-by: Chunguang Wu
> ---
> drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
> b/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
> index 4d1f9bf..d06a747 100644
> --- a/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
> +++ b/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
> @@ -292,7 +292,7 @@ static void halbtc_leave_lps(struct btc_coexist
> *btcoexist)
>
> if (ap_enable) {
> RT_TRACE(rtlpriv, COMP_BT_COEXIST, DBG_DMESG,
> - "%s()<--dont leave lps under AP mode\n", __func__);
> + "%s()<--don't leave lps under AP mode\n", __func__);
> return;
> }
>
> @@ -315,7 +315,7 @@ static void halbtc_enter_lps(struct btc_coexist
> *btcoexist)
>
> if (ap_enable) {
> RT_TRACE(rtlpriv, COMP_BT_COEXIST, DBG_DMESG,
> - "%s()<--dont enter lps under AP mode\n", __func__);
> + "%s()<--don't enter lps under AP mode\n", __func__);
> return;
> }
Patch is corrupted and can not be applied :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] Staging: rts5208: rtsx_card.c: Fixed all braces issues of the file
On Tue, Oct 02, 2018 at 07:11:26PM -0400, Maxime Desroches wrote: > Fixed all the braces issues of the rtsx_card.c file > > Signed-off-by: Maxime Desroches > --- > drivers/staging/rts5208/rtsx_card.c | 96 +++-- > 1 file changed, 37 insertions(+), 59 deletions(-) None of the patches in this series applied to my staging-next tree at all :( Always work against either linux-next, or my staging-next tree when submitting patches so you do not duplicate work others have already done. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] STAGING/EMXX_UDC: emxx_udc.c: Fixed all meaningful sparse errors: 1. Added static to udc_controller 2. Added mising __iomem modifier to handle p_regs 3. Added missing le16_to_cpu
On Fri, Oct 05, 2018 at 01:04:27PM -0400, Carmeli Tamir wrote: > Signed-off-by: Tamir Carmeli > --- > drivers/staging/emxx_udc/Makefile | 2 +- > drivers/staging/emxx_udc/emxx_udc.c | 69 > +++-- > drivers/staging/emxx_udc/emxx_udc.h | 2 +- > 3 files changed, 38 insertions(+), 35 deletions(-) Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - Your patch did many different things all at once, making it difficult to review. All Linux kernel patches need to only do one thing at a time. If you need to do multiple things (such as clean up all coding style issues in a file/driver), do it in a sequence of patches, each one doing only one thing. This will make it easier to review the patches to ensure that they are correct, and to help alleviate any merge issues that larger patches can cause. - You did not specify a description of why the patch is needed, or possibly, any description at all, in the email body. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/SubmittingPatches for what is needed in order to properly describe the change. - You did not write a descriptive Subject: for the patch, allowing Greg, and everyone else, to know what this patch is all about. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/SubmittingPatches for what a proper Subject: line should look like. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: emxx_udc: Remove unused device_desc declaration
On Fri, Oct 05, 2018 at 02:05:45PM -0700, Nick Desaulniers wrote:
> On Wed, Oct 3, 2018 at 10:56 PM Nathan Chancellor
> wrote:
> >
> > Clang warns:
> >
> > drivers/staging/emxx_udc/emxx_udc.c:1373:37: warning: variable
> > 'device_desc' is not needed and will not be emitted
> > [-Wunneeded-internal-declaration]
> > static struct usb_device_descriptor device_desc = {
> > ^
> > 1 warning generated.
> >
> > This definition hasn't been attached to anything since the driver was
> > introduced in commit 33aa8d45a4fe ("staging: emxx_udc: Add Emma Mobile
> > USB Gadget driver") and neither GCC nor Clang emit any reference to the
> > variable in the final assembly. The only reason GCC doesn't warn about
> > this variable being unused is the sizeof function.
> >
> > Reported-by: Nick Desaulniers
> > Signed-off-by: Nathan Chancellor
> > ---
> >
> > This seems kind of wrong given this is a USB driver but there isn't an
> > instance of a platform_driver in the kernel tree having a usb device
> > descriptor declaration so I'm unsure of how to handle this warning aside
> > from just removing the definition but I'm certainly open to suggestions.
>
> In drivers under drivers/usb/gadget/legacy/{ether|mass_storage|hid}.c,
> it seems that addresses of instances of `struct usb_device_descriptor`
> are stored in instances of `struct usb_composite_driver eth_driver`
> that are passed to module_usb_composite_driver().
>
> drivers/staging/emxx_udc/emxx_udc.c doesn't mention anything about
> being a composite driver, and I don't know if there are multiple
> devices to warrant a composite driver? Composite seems to imply "more
> than one gadget" while the path to drivers using this interface under
> drivers/usb/gadget/legacy/ seem to imply there's a modern (non-legacy)
> usb gadget interface that could potentially be used instead.
>
> If this was never intended to be a composite usb driver, or there's
> some reason why it doesn't make sense for it to be one, then this code
> is likely dead and your fix is correct. If it's not, maybe folks who
> know more about the USB interfaces have another solution to make this
> a composite usb driver?
I'll take the patch, the code looks wrong, it should not be needed here.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] STAGING/EMXX_UDC: Fixed all meaningful sparse errors.
On Sat, Oct 06, 2018 at 03:13:39AM -0400, Carmeli Tamir wrote: > Fixed all meaningful sparse errors: > 1. Added static to udc_controller > 2. Added mising __iomem modifier to handle p_regs > 3. Added missing le16_to_cpu > > Signed-off-by: Tamir Carmeli > --- > Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - Your patch did many different things all at once, making it difficult to review. All Linux kernel patches need to only do one thing at a time. If you need to do multiple things (such as clean up all coding style issues in a file/driver), do it in a sequence of patches, each one doing only one thing. This will make it easier to review the patches to ensure that they are correct, and to help alleviate any merge issues that larger patches can cause. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] staging: android: ion: Add per-heap counters
On Sun, Sep 30, 2018 at 06:24:52PM +0300, Alexey Skidanov wrote: > Heap statistics have been removed and currently even basics statistics > are missing. > > This patch creates per heap debugfs directory /sys/kernel/debug/ > and adds the following counters: > - the number of allocated buffers; > - the number of allocated bytes; > - the number of allocated bytes watermark. > > Signed-off-by: Alexey Skidanov > Acked-by: Laura Abbott > --- > > v3: > Removed debugfs_create_dir() return value checking > v4: > Added spinlock to protect heap statistics > > drivers/staging/android/ion/ion.c | 50 > --- > drivers/staging/android/ion/ion.h | 10 +++- > 2 files changed, 51 insertions(+), 9 deletions(-) This patch fails to apply to my staging-next branch :( Please fix up and resend. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/11] staging:rtl8192u: Removed commented out include - Style
On Sun, Oct 07, 2018 at 10:40:16PM +0100, John Whitmore wrote: > Remove commented out #include directive. > > Additionally shorted a block comment to clear the checkpatch issue > with line length. You really should only do one type of thing per patch, but I'll let this one go... ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 06/11] staging:rtl8192u: Rewrite test for null - Style
On Sun, Oct 07, 2018 at 10:40:20PM +0100, John Whitmore wrote: > Rewrite a test for NULL to comply with the coding style and clear the > checkpatch issue. > > This is a coding style change which should not impact runtime > code execution. > > Signed-off-by: John Whitmore > --- > drivers/staging/rtl8192u/ieee80211/ieee80211_module.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) This patch did not apply :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net-next] hv_netvsc: fix vf serial matching with pci slot info
On Thu, Oct 11, 2018 at 08:14:34PM +, Haiyang Zhang wrote:
> From: Haiyang Zhang
>
> The VF device's serial number is saved as a string in PCI slot's
> kobj name, not the slot->number. This patch corrects the netvsc
> driver, so the VF device can be successfully paired with synthetic
> NIC.
>
> Fixes: 00d7ddba1143 ("hv_netvsc: pair VF based on serial number")
> Signed-off-by: Haiyang Zhang
> ---
> drivers/net/hyperv/netvsc_drv.c | 15 +++
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 9bcaf204a7d4..8121ce34a39f 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -2030,14 +2030,15 @@ static void netvsc_vf_setup(struct work_struct *w)
> rtnl_unlock();
> }
>
> -/* Find netvsc by VMBus serial number.
> - * The PCI hyperv controller records the serial number as the slot.
> +/* Find netvsc by VF serial number.
> + * The PCI hyperv controller records the serial number as the slot kobj name.
> */
> static struct net_device *get_netvsc_byslot(const struct net_device
> *vf_netdev)
> {
> struct device *parent = vf_netdev->dev.parent;
> struct net_device_context *ndev_ctx;
> struct pci_dev *pdev;
> + u32 serial;
>
> if (!parent || !dev_is_pci(parent))
> return NULL; /* not a PCI device */
> @@ -2048,16 +2049,22 @@ static struct net_device *get_netvsc_byslot(const
> struct net_device *vf_netdev)
> return NULL;
> }
>
> + if (kstrtou32(pdev->slot->kobj.name, 10, &serial)) {
kobject_name()?
And that feels _very_ fragile to me. This is now an api that you are
guaranteeing will never change?
Good luck with that!
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/5] Drivers: hv: kvp: Use %u to print U32
On Wed, Oct 17, 2018 at 03:14:05AM +, k...@linuxonhyperv.com wrote: > From: Dexuan Cui > > I didn't find a real issue. Let's just make it consistent with the > next "case REG_U64:" where %llu is used. > > Signed-off-by: Dexuan Cui > Cc: K. Y. Srinivasan > Cc: Haiyang Zhang > Cc: Stephen Hemminger > Cc: Why is this a stable patch? confused, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/5] Drivers: hv: vmbus: Get rid of unnecessary state in hv_context
On Wed, Oct 17, 2018 at 03:14:02AM +, k...@linuxonhyperv.com wrote: > From: "K. Y. Srinivasan" > > Currently we are replicating state in struct hv_context that is unnecessary - > this state can be retrieved from the hypervisor. Furthermore, this is a > per-cpu > state that is being maintained as a global state in struct hv_context. > Get rid of this state in struct hv_context. > > Reply-To: k...@microsoft.com Why is this here? greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
On Wed, Oct 17, 2018 at 03:14:04AM +, k...@linuxonhyperv.com wrote:
> From: Dexuan Cui
>
> In kvp_send_key(), we do need call process_ib_ipinfo() if
> message->kvp_hdr.operation is KVP_OP_GET_IP_INFO, because it turns out
> the userland hv_kvp_daemon needs the info of operation, adapter_id and
> addr_family. With the incorrect fc62c3b1977d, the host can't get the
> VM's IP via KVP.
>
> And, fc62c3b1977d added a "break;", but actually forgot to initialize
> the key_size/value in the case of KVP_OP_SET, so the default key_size of
> 0 is passed to the kvp daemon, and the pool files
> /var/lib/hyperv/.kvp_pool_* can't be updated.
>
> This patch effectively rolls back the previous fc62c3b1977d, and
> correctly fixes the "this statement may fall through" warnings.
>
> This patch is tested on WS 2012 R2 and 2016.
>
> Fixes: fc62c3b1977d ("Drivers: hv: kvp: Fix two "this statement may fall
> through" warnings")
> Signed-off-by: Dexuan Cui
> Cc: K. Y. Srinivasan
> Cc: Haiyang Zhang
> Cc: Stephen Hemminger
> Cc:
> Signed-off-by: K. Y. Srinivasan
> ---
> drivers/hv/hv_kvp.c | 26 ++
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
> index a7513a8a8e37..9fbb15c62c6c 100644
> --- a/drivers/hv/hv_kvp.c
> +++ b/drivers/hv/hv_kvp.c
> @@ -353,6 +353,9 @@ static void process_ib_ipinfo(void *in_msg, void
> *out_msg, int op)
>
> out->body.kvp_ip_val.dhcp_enabled = in->kvp_ip_val.dhcp_enabled;
>
> + __attribute__ ((fallthrough));
The comment should be sufficient for this, right? I haven't seen many
uses of this attribute before, how common is it?
> +
> + case KVP_OP_GET_IP_INFO:
> utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.adapter_id,
> MAX_ADAPTER_ID_SIZE,
> UTF16_LITTLE_ENDIAN,
> @@ -405,7 +408,11 @@ kvp_send_key(struct work_struct *dummy)
> process_ib_ipinfo(in_msg, message, KVP_OP_SET_IP_INFO);
> break;
> case KVP_OP_GET_IP_INFO:
> - /* We only need to pass on message->kvp_hdr.operation. */
> + /*
> + * We only need to pass on the info of operation, adapter_id
> + * and addr_family to the userland kvp daemon.
> + */
> + process_ib_ipinfo(in_msg, message, KVP_OP_GET_IP_INFO);
> break;
> case KVP_OP_SET:
> switch (in_msg->body.kvp_set.data.value_type) {
> @@ -446,9 +453,9 @@ kvp_send_key(struct work_struct *dummy)
>
> }
>
> - break;
> -
> - case KVP_OP_GET:
> + /*
> + * The key is always a string - utf16 encoding.
> + */
> message->body.kvp_set.data.key_size =
> utf16s_to_utf8s(
> (wchar_t *)in_msg->body.kvp_set.data.key,
> @@ -456,6 +463,17 @@ kvp_send_key(struct work_struct *dummy)
> UTF16_LITTLE_ENDIAN,
> message->body.kvp_set.data.key,
> HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1;
> +
> + break;
> +
> + case KVP_OP_GET:
> + message->body.kvp_get.data.key_size =
> + utf16s_to_utf8s(
> + (wchar_t *)in_msg->body.kvp_get.data.key,
> + in_msg->body.kvp_get.data.key_size,
> + UTF16_LITTLE_ENDIAN,
> + message->body.kvp_get.data.key,
> + HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1;
Worst indentation ever :(
Yeah, I know it follows the others above it, but you should reconsider
it in the future...
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] Tools: hv: kvp: Fix a warning of buffer overflow with gcc 8.0.1
On Wed, Oct 17, 2018 at 03:14:06AM +, k...@linuxonhyperv.com wrote: > From: Dexuan Cui > > The patch fixes: > > hv_kvp_daemon.c: In function 'kvp_set_ip_info': > hv_kvp_daemon.c:1305:2: note: 'snprintf' output between 41 and 4136 bytes > into a destination of size 4096 > > Signed-off-by: Dexuan Cui > Cc: K. Y. Srinivasan > Cc: Haiyang Zhang > Cc: Stephen Hemminger > Cc: > Signed-off-by: K. Y. Srinivasan > --- > tools/hv/hv_kvp_daemon.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c > index bbb2a8ef367c..b7c2cf7eaba5 100644 > --- a/tools/hv/hv_kvp_daemon.c > +++ b/tools/hv/hv_kvp_daemon.c > @@ -1176,7 +1176,7 @@ static int kvp_set_ip_info(char *if_name, struct > hv_kvp_ipaddr_value *new_val) > int error = 0; > char if_file[PATH_MAX]; > FILE *file; > - char cmd[PATH_MAX]; > + char cmd[PATH_MAX * 2]; Overkill? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[GIT PULL] Staging/IIO driver patches for 4.20-rc1
The following changes since commit 7876320f88802b22d4e2daf7eb027dd14175a0f8: Linux 4.19-rc4 (2018-09-16 11:52:37 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git tags/staging-4.20-rc1 for you to fetch changes up to 4ab7e05dd070600833680bd318d6d962f010caa2: staging: gasket: Fix sparse "incorrect type in assignment" warnings. (2018-10-19 21:12:29 +0200) Staging/IIO patches for 4.20-rc1 Here is the big staging and IIO driver pull request for 4.20-rc1. There are lots of things here, we ended up adding more lines than removing, thanks to a large influx of Comedi National Instrument device support. Someday soon we need to get comedi out of staging... Other than the comedi drivers, the "big" things here are: - new iio drivers - delete dgnc driver (no one used it and no one had the hardware anymore) - vbox driver updates and fixes - erofs fixes - tons and tons of tiny checkpatch fixes for almost all staging drivers All of these have been in linux-next, with the last few happening a bit "late" due to them getting stuck on my laptop during travel to the Mantainers summit. When merging with your tree, there will be 2 merge conflicts, both files will be simple to resolve, just delete them :) Signed-off-by: Greg Kroah-Hartman Ajay Singh (55): staging: wilc1000: move 'wilc_enable_ps' global variable into 'wilc' struct staging: wilc1000: move 'aging_timer' static variable to wilc_priv struct staging: wilc1000: fix to use correct index to free scanned info in clear_shadow_scan() staging: wilc1000: remove unnecessary NULL check in clear_shadow_scan() staging: wilc1000: moved last_scanned_shadow & last_scanned_cnt to wilc_priv struct staging: wilc1000: move during_ip_timer & wilc_optaining_ip to 'wilc_vif' struct staging: wilc1000: remove unused variable 'op_ifcs' staging: wilc1000: avoid use of extra 'if' condition in wilc_init() staging: wilc1000: move static variable clients_count to 'wilc' structure staging: wilc1000: refactor code to avoid use of wilc_set_multicast_list global staging: wilc1000: move hif_workqueue static variables to 'wilc' structure staging: wilc1000: move 'periodic_rssi' as part of 'wilc_vif' struct staging: wilc1000: rename 'dummy_statistics' variable to 'periodic_stat' staging: wilc1000: move 'rcv_assoc_resp' as part of hif_drv staging: wilc1000: refactor tcp_process() to avoid extra leading tabs staging: wilc1000: use lowercase for get_BSSID() and HIL variable staging: wilc1000: move tcp_ack_filter algo related variables to 'wilc_vif' struct staging: wilc1000: avoid line over 80 chars in wilc_wlan_txq_filter_dup_tcp_ack() staging: wilc1000: use short names to fix over 80 issue in tcp_process() staging: wilc1000: remove unused code to set and get IP address staging: wilc1000: move 'chip_ps_state' static variable as part of 'wilc' struct staging: wilc1000: move 'wilc_connecting' static variable to 'wilc_vif' struct staging: wilc1000: remove unnecessary static variable 'p2p_listen_state' staging: wilc1000: refactor code to move initilization in wilc_netdev_init() staging: wilc1000: refactor wilc_netdev_init() to handle memory free in error path staging: wilc1000: remove handle_hif_exit_work() function staging: wilc1000: change return type to 'void' for wilc_frame_register() staging: wilc1000: change return type to 'void' for wilc_wlan_set_bssid() staging: wilc1000: change return type to 'void' for lock init & deinit functions staging: wilc1000: change return type to 'void' for wilc_deinit_host_int() staging: wilc1000: change return type to 'void' for wilc_wfi_deinit_mon_interface() staging: wilc1000: use 'void' return type for host_int_get_assoc_res_info() staging: wilc1000: use 'void' return for wilc_wlan_txq_add_to_head() staging: wilc1000: change return type to 'void' tcp ack filter functions staging: wilc1000: use 'void' return for wilc_wlan_txq_filter_dup_tcp_ack() staging: wilc1000: change return type to 'void' for wilc_wlan_cfg_indicate_rx() staging: wilc1000: refactor wilc_wlan_parse_info_frame() function staging: wilc1000: set default value of cfg response type in wilc_wlan_cfg_indicate_rx() staging: wilc1000: changes 'val' type to u8 in wilc_cfg_byte struct staging: wilc1000: remove unused wid type values staging: wilc1000: remove unused wid from cfg struct staging: wilc1000: refactor code to remove 'mac_status' from 'wilc_mac_cfg' struct staging: wilc1000: refactor code to avoid static variables for config parameters staging: wilc1000: rename 'wilc_mac_cfg' struct to 'wilc_cfg_str_v
Re: [PATCH] staging: fix some typo
On Thu, Nov 01, 2018 at 11:48:38AM -0400, Yangtao Li wrote: > Signed-off-by: Yangtao Li > --- > drivers/staging/rtl8712/rtl871x_cmd.h | 2 +- > drivers/staging/rtl8723bs/include/rtw_cmd.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) No changelog? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: update wilc1000 driver maintainer ids
On Tue, Oct 30, 2018 at 05:53:40AM +, ajay.kat...@microchip.com wrote: > From: Ajay Singh > > We would like to update the maintainer email id's for wilc1000 driver. > > Signed-off-by: Ajay Singh > Signed-off-by: Adham Abozaeid It would be good to get the current maintainer's signed off by on this so I know that they also agree with this change. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/4] staging: wilc1000: remove unused flags in handle_cfg_param()
On Tue, Oct 30, 2018 at 04:22:55AM +, adham.aboza...@microchip.com wrote: > From: Adham Abozaeid > > handle_cfg_param() receives a bit map that describes what to be changed. > Some of these bits flags aren't referred to from elsewhere and can be > removed. > > Signed-off-by: Adham Abozaeid > --- > drivers/staging/wilc1000/host_interface.c | 216 -- > drivers/staging/wilc1000/host_interface.h | 29 --- > drivers/staging/wilc1000/wilc_wlan_if.h | 9 - > 3 files changed, 254 deletions(-) > mode change 100644 => 100755 drivers/staging/wilc1000/host_interface.c > mode change 100644 => 100755 drivers/staging/wilc1000/host_interface.h > mode change 100644 => 100755 drivers/staging/wilc1000/wilc_wlan_if.h Do not do Linux kernel testing on filesystems on windows machines :( You just changed the permissions on these files for no reason, which isn't ok at all :( Dropping this series from my queue. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: fix some typo
On Fri, Nov 02, 2018 at 08:53:32AM -0400, Yangtao Li wrote: > ture->true > > Signed-off-by: Yangtao Li > --- > Change in v2: > -add changelog > --- > drivers/staging/rtl8712/rtl871x_cmd.h | 2 +- > drivers/staging/rtl8723bs/include/rtw_cmd.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) One patch per driver please. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vt6655: fix small typo
On Thu, Nov 01, 2018 at 11:52:51AM -0400, Yangtao Li wrote: > Signed-off-by: Yangtao Li > --- > drivers/staging/vt6655/baseband.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I can not accept patches without any changelog text :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: vchi: Add license id and change int type
On Tue, Nov 06, 2018 at 10:07:38PM -0200, andrealm...@riseup.net wrote: > From: André Almeida > > Fix the following checkpatch issues: > > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 > CHECK: Prefer kernel type 's32' over 'int32_t' > > Signed-off-by: André Almeida > > --- > > Hello! This is my first patch to Linux Kernel. Let me know any feedback > --- > drivers/staging/vc04_services/interface/vchi/vchi_mh.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h > b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h > index 198bd076b666..42fc0c693d43 100644 > --- a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h > +++ b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h > @@ -1,5 +1,5 @@ > -/** Why change this line? > - * Copyright (c) 2010-2012 Broadcom. All rights reserved. > +/* SPDX-License-Identifier: GPL-2.0 */ That is not the correct license for this file. Please never make a change like this unless you really know your license information. > +/* Copyright (c) 2010-2012 Broadcom. All rights reserved. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions > @@ -36,7 +36,7 @@ > > #include > > -typedef int32_t VCHI_MEM_HANDLE_T; > +typedef s32 VCHI_MEM_HANDLE_T; You are doing multiple things in the same patch, which isn't allowed. Please break all patches up into one "logical" thing per patch and send a patch series. Also, are you sure this type change is correct? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: wilc1000: update wilc1000 driver maintainer ids
On Wed, Nov 07, 2018 at 05:05:01AM +, ajay.kat...@microchip.com wrote: > From: Ajay Singh > > We would like to update the maintainer email id's for wilc1000 driver. > > Signed-off-by: Aditya Shankar > Signed-off-by: Ganesh Krishna > Signed-off-by: Adham Abozaeid > Signed-off-by: Ajay Singh Not that I do not trust you, but I would really like to see an email directly from the existing maintainers that they did sign off on this or at least ack it. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: RFC: staging: gasket: re-implement using UIO
On Tue, Nov 06, 2018 at 04:20:49PM -0800, Todd Poynor wrote: > On Mon, Sep 10, 2018 at 8:28 AM Ahmed S. Darwish wrote: > > > > The gasket in-kernel framework, recently introduced under staging, > > re-implements what is already long-time provided by the UIO > > subsystem, with extra PCI BAR remapping and MSI conveniences. > > > > Before moving it out of staging, make sure we add the new bits to > > the UIO framework instead, then transform its signle client, the > > Apex driver, to a proper UIO driver (uio_driver.h). > > > > Link: https://lkml.kernel.org/r/20180828103817.GB1397@do-kernel > > So I'm looking at this for reals now. The BAR mapping stuff is > straightforward with the existing framework. Everything else could be > done outside of UIO via the existing device interface, but figured I'd > collect any opinions about adding the new bits to UIO. > > The Apex device has 13 MSIX interrupts. UIO does one IRQ per device. > The PRUSS driver registers 8 instances of the UIO device with > identical memory mappings but individual IRQs for its 8 interrupts. > Currently gasket has userspace pass down an eventfd (via ioctl) for > each interrupt it wants to watch. Is there interest in modifying UIO > to handle multiple IRQs in some perhaps similar fashion? > > Speaking of ioctls, are those allowed here, or is sysfs or something > else always required? The aforementioned multiple IRQ stuff probably > maps nicely to sysfs (there's a small number of them easily > represented as attributes), while DMA buffer mappings seem more > problematic, but maybe somebody's thought of a good way to represent > these already. Adding ioctls opens up a huge can of worms where each driver would probably want to do their own mess and then we have to constantly audit the mess all of the time. What do you really need to do in an ioctl? > And then we need to map buffers to our device. We could probably > implement this via an IOMMU driver API for our custom MMU and hook > that up to generic IOMMU support for UIO, which sounds like something > a lot of drivers could use. You need to map buffers from what to what? UIO is for pass-through memory that your device exposes to userspace through mmap, what more do you need that the existing api does not give you? > There's a few other tidbits the driver does, including allocating > coherent memory for userspace to share with the device, but that's > probably enough for now. > > If anybody wants to squash any of the above as a non-starter for UIO > or point things in a different direction, it's appreciated. Patches are always the best way to do review, we generally do not have time or bandwidth to do "what do you think of my design" ideas without real code behind it, sorry. That way we know you really did try to do something and you have something that starts to work for you. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: mt7621-mmc: Add blank line after declaration
On Tue, Nov 06, 2018 at 10:42:42PM -0200, nico...@autobyte.com.br wrote:
> From: Nícolas F. R. A. Prado
>
> Correct the following warning from checkpatch.pl:
>
> WARNING: Missing a blank line after declarations
> + struct msdc_host *host = mmc_priv(mmc);
> + msdc_pm(state, (void *)host);
>
> Signed-off-by: Nícolas F. R. A. Prado
>
> ---
>
> Hi, this is my first commit. Let me know if there's anything I can do
> better.
> ---
> drivers/staging/mt7621-mmc/sd.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
> index 0379f9c96..7b66f9b0a 100644
> --- a/drivers/staging/mt7621-mmc/sd.c
> +++ b/drivers/staging/mt7621-mmc/sd.c
> @@ -1797,6 +1797,7 @@ static void msdc_drv_pm(struct platform_device *pdev,
> pm_message_t state)
>
> if (mmc) {
> struct msdc_host *host = mmc_priv(mmc);
> +
> msdc_pm(state, (void *)host);
> }
This patch is fine, and I'll take it, but can't this be written a lot
simpler:
if (mmc)
msdc_pm(state, mmc_priv(mmc));
that looks better, don't you think?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: sm750fb: Add spaces around '+'
On Tue, Nov 06, 2018 at 10:28:04PM -0200, Laís Pessine do Carmo wrote: > Add spaces around '+' to correct the following checkpath.pl warnings: > > WARNING: line over 80 characters > + write_dpPort(accel, *(unsigned int *)(pSrcbuf + > (j * 4))); > > WARNING: line over 80 characters > + memcpy(ajRemain, pSrcbuf+ul4BytesPerScan, > ulBytesRemain); No need to line-wrap these warning lines next time. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: emxx_udc: Added static modifier to udc_controller
On Thu, Nov 01, 2018 at 03:58:57PM -0400, Carmeli Tamir wrote: > Added static modifier to the udc_controller, since it's only > required within emxx_udc.c. > Previously posted without any feedback, now updated according to master. > > Signed-off-by: Carmeli Tamir > --- > drivers/staging/emxx_udc/emxx_udc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I do not see patch 3/3 in this series. Can you resend the whole series, properly threaded, so that I know what to apply? I think you send this one already and I have no idea what to do here, sorry. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gasket: Use octal permissions instead of symbolic.
On Wed, Nov 07, 2018 at 05:41:29PM +0530, Rohit Sarkar wrote:
> On Tue, Nov 06, 2018 at 09:12:45PM +0100, Greg KH wrote:
> > On Tue, Nov 06, 2018 at 04:33:47PM +0530, Rohit Sarkar wrote:
> > > Replace S_IRUGO with 0444. Issue found by checkpatch.
> > >
> > > Signed-off-by: Rohit Sarkar
> > > ---
> > > drivers/staging/gasket/gasket_sysfs.h | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/gasket/gasket_sysfs.h
> > > b/drivers/staging/gasket/gasket_sysfs.h
> > > index 151e8edd28ea..60590ea4b760 100644
> > > --- a/drivers/staging/gasket/gasket_sysfs.h
> > > +++ b/drivers/staging/gasket/gasket_sysfs.h
> > > @@ -40,7 +40,7 @@
> > > */
> > > #define GASKET_END_OF_ATTR_ARRAY
> > > \
> > > { \
> > > - .attr = __ATTR(GASKET_ARRAY_END_TOKEN, S_IRUGO, NULL, NULL), \
> > > + .attr = __ATTR(GASKET_ARRAY_END_TOKEN, 0444, NULL, NULL), \
> > > .data.attr_type = 0, \
> >
> > No, this horrid #define needs to just be removed entirely, it's not
> > really needed at all.
> >
> > > }
> > >
> > > @@ -75,7 +75,7 @@ struct gasket_sysfs_attribute {
> > >
> > > #define GASKET_SYSFS_RO(_name, _show_function, _attr_type)
> > > \
> > > { \
> > > - .attr = __ATTR(_name, S_IRUGO, _show_function, NULL), \
> > > + .attr = __ATTR(_name, 0444, _show_function, NULL), \
> >
> > __ATTR_RO() please.
> >
> > Or just fix it up so this isn't needed at all, the sysfs usage in this
> > driver needs major work (i.e. almost deleted entirely...)
> >
> > thanks,
> >
> > greg k-h
>
> Hey,
> I am relatively new to the community and I was searching for things
> to work on. I would love to help with this. Do you think this would be
> a good task for me to get started with as I cannot seem to guage the
> difficulty of this.
If you do not understand how "raw" sysfs files work, no, I would not
recommend this. Also, this whole driver is hopefully going to be
deleted soon by using a UIO driver.
But, that deletion might take a while, so if you really are interested,
read up on how kobjects and sysfs attributes work and you might be able
to fix this up before the code gets deleted, it's up to you :)
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 2/4] staging: wilc1000: validate cfg parameters before scheduling the work
On Tue, Nov 06, 2018 at 12:01:18AM +, adham.aboza...@microchip.com wrote: > From: Adham Abozaeid > > Validate cfg parameters after being called by cfg80211 in set_wiphy_params > before scheduling the work executed in handle_cfg_param > > Signed-off-by: Adham Abozaeid > --- > drivers/staging/wilc1000/host_interface.c | 61 ++- > .../staging/wilc1000/wilc_wfi_cfgoperations.c | 50 --- > 2 files changed, 62 insertions(+), 49 deletions(-) Please fix this up so that the 0-day bot does not complain about it. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] staging: wlan-ng: reformatting in hfa384x.h to fit 80 character limit
On Wed, Nov 07, 2018 at 08:09:19PM +, Tim Collier wrote:
> Reformat lines over 80 characters in hfa384x.h to resolve "line over
> 80 characters" warnings reported by checkpatch.
>
> Signed-off-by: Tim Collier
> ---
> drivers/staging/wlan-ng/hfa384x.h | 26 ++
> 1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/wlan-ng/hfa384x.h
> b/drivers/staging/wlan-ng/hfa384x.h
> index 992ebaa1071f..4f2177d1e1bc 100644
> --- a/drivers/staging/wlan-ng/hfa384x.h
> +++ b/drivers/staging/wlan-ng/hfa384x.h
> @@ -517,7 +517,8 @@ struct hfa384x_tx_frame {
> HFA384x_TXSTATUS_DISCON | HFA384x_TXSTATUS_AGEDERR | \
> HFA384x_TXSTATUS_RETRYERR))
>
> -#define HFA384x_TX_SET(v, m, s) u16)(v)) << ((u16)(s))) &
> ((u16)(m)))
> +#define HFA384x_TX_SET(v, m, s) \
> + u16)(v)) << ((u16)(s))) & ((u16)(m)))
Ick, can't this look better as:
#define HFA384x_TX_SET(v, m, s) u16)(v)) << ((u16)(s))) &
((u16)(m)))
No need for the tab after "define", and splitting this to a new line
just makes it harder to read.
Odds are this macro can just be replaced with something standard as
well, that looks like a common bit mask/set macro, right?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 1/3] staging: wilc1000: validate cfg parameters before scheduling the work
On Thu, Nov 08, 2018 at 09:50:25PM +, adham.aboza...@microchip.com wrote: > From: Adham Abozaeid > > From: Adham Abozaeid Twice? Something went wrong on your side, for all of these patches :( Please fix up and resend. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers/android/binder.c: Remove duplicate header
On Fri, Nov 09, 2018 at 09:44:25AM +0530, Brajeswar Ghosh wrote:
> Remove binder_trace.h which is included more than once
>
> Signed-off-by: Brajeswar Ghosh
> ---
> drivers/android/binder.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index cb30a524d16d..719f35a5c04b 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5852,6 +5852,5 @@ static int __init binder_init(void)
> device_initcall(binder_init);
>
> #define CREATE_TRACE_POINTS
> -#include "binder_trace.h"
>
> MODULE_LICENSE("GPL v2");
Are you sure about this? Did you test the tracepoint functionality
after removing that line? I think you just broke it :(
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] binder: fix race that allows malicious free of live buffer
On Tue, Nov 06, 2018 at 03:55:32PM -0800, Todd Kjos wrote: > Malicious code can attempt to free buffers using the > BC_FREE_BUFFER ioctl to binder. There are protections > against a user freeing a buffer while in use by the > kernel, however there was a window where BC_FREE_BUFFER > could be used to free a recently allocated buffer that > was not completely initialized. This resulted in a > use-after-free detected by KASAN with a malicious > test program. > > This window is closed by setting the buffer's > allow_user_free attribute to 0 when the buffer > is allocated or when the user has previously > freed it instead of waiting for the caller > to set it. The problem was that when the struct > buffer was recycled, allow_user_free was stale > and set to 1 allowing a free to go through. > > Signed-off-by: Todd Kjos > Acked-by: Arve Hjønnevåg No "stable" tag here? Any idea how far back the stable backporting should go, if any? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers/android/binder.c: Remove duplicate header
On Fri, Nov 09, 2018 at 10:40:14PM +0800, kbuild test robot wrote: > Hi Brajeswar, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on staging/staging-testing] > [also build test ERROR on v4.20-rc1 next-20181109] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Brajeswar-Ghosh/drivers-android-binder-c-Remove-duplicate-header/20181109-154717 > config: i386-allmodconfig (attached as .config) > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All errors (new ones prefixed by >>): Yeah, I was right :( Always test-build your patches. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 00/33] staging: mt7621-pci: Parse ports info from DT and other minor cleanups
On Sun, Nov 04, 2018 at 11:49:26AM +0100, Sergio Paracuellos wrote: > This patch series parse remaining port info from device tree storing > it in mt7621_pcie_port struct created for this. It also performs a lot > of cleanups to get the driver in a good shape to give it a try to get > mainlined. All of this changes are only compile-tested. Given the lack of responses here, I guess I'll just merge this and see what happens :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/16] staging: vchiq: dead code removal & misc fixes
On Sun, Nov 18, 2018 at 04:55:49PM +0100, Stefan Wahren wrote: > Hi Nicolas, > > > Nicolas Saenz Julienne hat am 14. November 2018 um > > 13:59 geschrieben: > > > > > > Hi All, > > > > This series was written in parallel with reading and understanding the > > vchiq code. So excuse me for the lack of logic in the sequence of > > patches. > > > > The main focus was to delete as much code as possible, I've counted > > around 550 lines, which is not bad. Apart from that there are some > > patches enforcing proper kernel APIs usage. > > > > The only patch that really changes code is the > > vchiq_ioc_copy_element_data() rewrite. > > > > The last commit updates the TODO list with some of my observations, I > > realise some of the might be a little opinionated. If anything it's > > going to force a discussion on the topic, which is nice. > > > > It was developed on top of the latest linux-next, and was tested on a > > RPIv3B+ with audio, video and running vchiq_test. > > > > Regards, > > Nicolas > > > > without a changelog i won't start a review. What do you mean by this? The individual patches have a "changelog" in them, this is just a summary of the overall changes. What do you feel is missing here? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/7] staging: mt7621-pci: some fixes after test previous series
On Sun, Nov 25, 2018 at 07:59:48AM +0100, Sergio Paracuellos wrote: > On Sat, Nov 24, 2018 at 9:05 PM NeilBrown wrote: > > > > On Sat, Nov 24 2018, Sergio Paracuellos wrote: > > > > > Previous cleanup series was added to the staging tree without any > > > testing. After get testing feedback some issues appear and this patch > > > series should make the driver works properly again. > > > > > > Previous series are here: > > > * > > > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/128200.html > > > > > > Feedback after testing from Neil Brown is here: > > > * > > > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/129031.html > > > > > > There is one issue with chip revision and reset lines where those > > > are inverted. I achieve this including some wrappers for checking > > > the version in driver code and use proper reset_control_* functions. > > > I checked the 'arch/mips/ralink/reset.c' and think a good way to add > > > a quirk there but I ended up handling those inside the driver. > > > > > > Changes in v2: > > > - PATCH 7: In commit message: 's/mt7621-pcie/mt7621-pci/g' > > > > > > Hope this helps. > > > > > > Best regards, > > > Sergio Paracuellos > > > > > > Sergio Paracuellos (7): > > > staging: mt7621-pci: avoid mapping sysctls registers > > > staging: mt7621-dts: remove sysctl registers from pcie bindings > > > staging: mt7621-pci: dt-bindings: update bindings doc removing sysctls > > > registers > > > staging: mt7621-pci: fix reset lines for each pcie port > > > staging: mt7621-pci: avoid using clk_* operations > > > > all above: > >Tested-by: NeilBrown > > > > Thanks! > > > > > staging: mt7621-dts: remove clocks for pcie bindings > > > staging: mt7621-pci: dt-bindings: update bindings doc removing clocks > > > > I don't think we really want these - at least, not yet. > > The clock numbers do appear in the driver as > > > > #define PCIE_PORT_CLK_EN(x) BIT(24 + (x)) > > > > and > > rt_sysc_m32(PCIE_PORT_CLK_EN(slot), 0, RALINK_CLKCFG1); > > Maybe that can be made generic... > > > > It is odd that the driver disables the clock, but never enables it. > > There is other clock-related code in the pci driver. Maybe it should > > just stay there, maybe it should go to a clock driver. I don't really > > know. But this dts stuff "looks" right, so I'd rather leave it until we > > know that it will be useless. > > Ok! I understand your point. Because those are the last two in the series, I > think there is no need to send v3 of this series without them. Greg, let me > know > if you prefer me to send v3 without last two patches. I'll just drop the last two from my queue, no need to resend. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] arm64: hyperv: Add core Hyper-V include files
On Thu, Nov 22, 2018 at 03:10:56AM +, k...@linuxonhyperv.com wrote: > --- /dev/null > +++ b/arch/arm64/include/asm/hyperv-tlfs.h > @@ -0,0 +1,338 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +/* > + * This file contains definitions from the Hyper-V Hypervisor Top-Level > + * Functional Specification (TLFS): > + * > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fvirtualization%2Fhyper-v-on-windows%2Freference%2Ftlfs&data=02%7C01%7Ckys%40microsoft.com%7Cc831a45fd63e4a4b083908d641216aa8%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636768009113747528&sdata=jRSrs9ZWXdmeS7LQUEpoSyUfBS7a5KLYy%2FolFdE2tI0%3D&reserved=0 "interesting" url :( Care to use a real one? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] arm64: hyperv: Add support for Hyper-V as a hypervisor
On Thu, Nov 22, 2018 at 03:10:57AM +, k...@linuxonhyperv.com wrote: > --- /dev/null > +++ b/arch/arm64/hyperv/hv_hvc.S > @@ -0,0 +1,54 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ Using SPDX is good, but: > + > +/* > + * Microsoft Hyper-V hypervisor invocation routines > + * > + * Copyright (C) 2018, Microsoft, Inc. > + * > + * Author : Michael Kelley > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published > + * by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or > + * NON INFRINGEMENT. See the GNU General Public License for more > + * details. No need for these two paragraphs at all. If you add them now, someone will just come along later and rip them out. Please don't include them in the first place. Not to mention the first paragraph doesn't make sense, think about "which" version 2 the FSF published (hint, there was 5 or 7 or so...) That's why we have one in the kernel tree for everyone to rely on. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] Drivers: hv: vmbus: Add hooks for per-CPU IRQ
On Thu, Nov 22, 2018 at 03:10:58AM +, k...@linuxonhyperv.com wrote:
> From: Michael Kelley
>
> Add hooks to enable/disable a per-CPU IRQ for VMbus. These hooks
> are in the architecture independent setup and shutdown paths for
> Hyper-V, and are needed by Linux guests on Hyper-V on ARM64. The
> x86/x64 implementation is null because VMbus interrupts on x86/x64
> don't use an IRQ.
>
> Signed-off-by: Michael Kelley
> Signed-off-by: K. Y. Srinivasan
> ---
> arch/x86/include/asm/mshyperv.h | 4
> drivers/hv/hv.c | 2 ++
> 2 files changed, 6 insertions(+)
>
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 0d6271cce198..8d97bd3a13a6 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -109,6 +109,10 @@ void hyperv_vector_handler(struct pt_regs *regs);
> void hv_setup_vmbus_irq(void (*handler)(void));
> void hv_remove_vmbus_irq(void);
>
> +/* On x86/x64, there isn't a real IRQ to be enabled/disable */
> +static inline void hv_enable_vmbus_irq(void) {}
> +static inline void hv_disable_vmbus_irq(void) {}
> +
> void hv_setup_kexec_handler(void (*handler)(void));
> void hv_remove_kexec_handler(void);
> void hv_setup_crash_handler(void (*handler)(struct pt_regs *regs));
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 166c2501de17..d0bb09a4bd73 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -307,6 +307,7 @@ int hv_synic_init(unsigned int cpu)
> hv_set_siefp(siefp.as_uint64);
>
> /* Setup the shared SINT. */
> + hv_enable_vmbus_irq();
> hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
>
> shared_sint.vector = HYPERVISOR_CALLBACK_VECTOR;
> @@ -434,6 +435,7 @@ int hv_synic_cleanup(unsigned int cpu)
> /* Disable the global synic bit */
> sctrl.enable = 0;
> hv_set_synic_state(sctrl.as_uint64);
> + hv_disable_vmbus_irq();
>
> return 0;
> }
> --
> 2.19.1
You created "null" hooks that do nothing, for no one in this patch
series, why?
Add them when they are needed not now. If I saw this code in the tree,
I would just go delete it as it is because it is not used at all.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels to two workqueues
On Mon, Nov 26, 2018 at 02:29:57AM +, k...@linuxonhyperv.com wrote:
> From: Dexuan Cui
>
> vmbus_process_offer() mustn't call channel->sc_creation_callback()
> directly for sub-channels, because sc_creation_callback() ->
> vmbus_open() may never get the host's response to the
> OPEN_CHANNEL message (the host may rescind a channel at any time,
> e.g. in the case of hot removing a NIC), and vmbus_onoffer_rescind()
> may not wake up the vmbus_open() as it's blocked due to a non-zero
> vmbus_connection.offer_in_progress, and finally we have a deadlock.
>
> The above is also true for primary channels, if the related device
> drivers use sync probing mode by default.
>
> And, usually the handling of primary channels and sub-channels can
> depend on each other, so we should offload them to different
> workqueues to avoid possible deadlock, e.g. in sync-probing mode,
> NIC1's netvsc_subchan_work() can race with NIC2's netvsc_probe() ->
> rtnl_lock(), and causes deadlock: the former gets the rtnl_lock
> and waits for all the sub-channels to appear, but the latter
> can't get the rtnl_lock and this blocks the handling of sub-channels.
>
> The patch can fix the multiple-NIC deadlock described above for
> v3.x kernels (e.g. RHEL 7.x) which don't support async-probing
> of devices, and v4.4, v4.9, v4.14 and v4.18 which support async-probing
> but don't enable async-probing for Hyper-V drivers (yet).
>
> The patch can also fix the hang issue in sub-channel's handling described
> above for all versions of kernels, including v4.19 and v4.20-rc3.
>
> So the patch should be applied to all the existing kernels.
>
> Fixes: 8195b1396ec8 ("hv_netvsc: fix deadlock on hotplug")
> Cc: sta...@vger.kernel.org
> Cc: Stephen Hemminger
> Cc: K. Y. Srinivasan
> Cc: Haiyang Zhang
> Signed-off-by: Dexuan Cui
> Signed-off-by: K. Y. Srinivasan
> ---
> drivers/hv/channel_mgmt.c | 188 +-
> drivers/hv/connection.c | 24 -
> drivers/hv/hyperv_vmbus.h | 7 ++
> include/linux/hyperv.h| 7 ++
> 4 files changed, 161 insertions(+), 65 deletions(-)
As Sasha pointed out, this patch does not even apply :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] Drivers: hv: vmbus: Add hooks for per-CPU IRQ
On Mon, Nov 26, 2018 at 07:47:41PM +, Michael Kelley wrote:
> From: Greg KH Monday, November 26, 2018 11:21 AM
>
> > > diff --git a/arch/x86/include/asm/mshyperv.h
> > > b/arch/x86/include/asm/mshyperv.h
> > > index 0d6271cce198..8d97bd3a13a6 100644
> > > --- a/arch/x86/include/asm/mshyperv.h
> > > +++ b/arch/x86/include/asm/mshyperv.h
> > > @@ -109,6 +109,10 @@ void hyperv_vector_handler(struct pt_regs *regs);
> > > void hv_setup_vmbus_irq(void (*handler)(void));
> > > void hv_remove_vmbus_irq(void);
> > >
> > > +/* On x86/x64, there isn't a real IRQ to be enabled/disable */
> > > +static inline void hv_enable_vmbus_irq(void) {}
> > > +static inline void hv_disable_vmbus_irq(void) {}
> > > +
> > > void hv_setup_kexec_handler(void (*handler)(void));
> > > void hv_remove_kexec_handler(void);
> > > void hv_setup_crash_handler(void (*handler)(struct pt_regs *regs));
> > > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> > > index 166c2501de17..d0bb09a4bd73 100644
> > > --- a/drivers/hv/hv.c
> > > +++ b/drivers/hv/hv.c
> > > @@ -307,6 +307,7 @@ int hv_synic_init(unsigned int cpu)
> > > hv_set_siefp(siefp.as_uint64);
> > >
> > > /* Setup the shared SINT. */
> > > + hv_enable_vmbus_irq();
> > > hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> > >
> > > shared_sint.vector = HYPERVISOR_CALLBACK_VECTOR;
> > > @@ -434,6 +435,7 @@ int hv_synic_cleanup(unsigned int cpu)
> > > /* Disable the global synic bit */
> > > sctrl.enable = 0;
> > > hv_set_synic_state(sctrl.as_uint64);
> > > + hv_disable_vmbus_irq();
> > >
> > > return 0;
> > > }
> > > --
> > > 2.19.1
> >
> > You created "null" hooks that do nothing, for no one in this patch
> > series, why?
> >
>
> hv_enable_vmbus_irq() and hv_disable_vmbus_irq() have non-null
> implementations in the ARM64 code in patch 2 of this series. The
> implementations are in the new file arch/arm64/hyperv/mshyperv.c.
> Or am I misunderstanding your point?
So you use a hook in an earlier patch and then add it in a later one?
Shouldn't you do it the other way around? As it is, the earlier patch
should not work properly, right?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/7] staging: mt7621-pci: some fixes after test previous series
On Mon, Nov 26, 2018 at 08:46:10PM +0100, Sergio Paracuellos wrote: > On Mon, Nov 26, 2018 at 4:28 PM Greg KH wrote: > > > > On Sun, Nov 25, 2018 at 07:59:48AM +0100, Sergio Paracuellos wrote: > > > On Sat, Nov 24, 2018 at 9:05 PM NeilBrown wrote: > > > > > > > > On Sat, Nov 24 2018, Sergio Paracuellos wrote: > > > > > > > > > Previous cleanup series was added to the staging tree without any > > > > > testing. After get testing feedback some issues appear and this patch > > > > > series should make the driver works properly again. > > > > > > > > > > Previous series are here: > > > > > * > > > > > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/128200.html > > > > > > > > > > Feedback after testing from Neil Brown is here: > > > > > * > > > > > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/129031.html > > > > > > > > > > There is one issue with chip revision and reset lines where those > > > > > are inverted. I achieve this including some wrappers for checking > > > > > the version in driver code and use proper reset_control_* functions. > > > > > I checked the 'arch/mips/ralink/reset.c' and think a good way to add > > > > > a quirk there but I ended up handling those inside the driver. > > > > > > > > > > Changes in v2: > > > > > - PATCH 7: In commit message: 's/mt7621-pcie/mt7621-pci/g' > > > > > > > > > > Hope this helps. > > > > > > > > > > Best regards, > > > > > Sergio Paracuellos > > > > > > > > > > Sergio Paracuellos (7): > > > > > staging: mt7621-pci: avoid mapping sysctls registers > > > > > staging: mt7621-dts: remove sysctl registers from pcie bindings > > > > > staging: mt7621-pci: dt-bindings: update bindings doc removing > > > > > sysctls > > > > > registers > > > > > staging: mt7621-pci: fix reset lines for each pcie port > > > > > staging: mt7621-pci: avoid using clk_* operations > > > > > > > > all above: > > > >Tested-by: NeilBrown > > > > > > > > Thanks! > > > > > > > > > staging: mt7621-dts: remove clocks for pcie bindings > > > > > staging: mt7621-pci: dt-bindings: update bindings doc removing > > > > > clocks > > > > > > > > I don't think we really want these - at least, not yet. > > > > The clock numbers do appear in the driver as > > > > > > > > #define PCIE_PORT_CLK_EN(x) BIT(24 + (x)) > > > > > > > > and > > > > rt_sysc_m32(PCIE_PORT_CLK_EN(slot), 0, RALINK_CLKCFG1); > > > > Maybe that can be made generic... > > > > > > > > It is odd that the driver disables the clock, but never enables it. > > > > There is other clock-related code in the pci driver. Maybe it should > > > > just stay there, maybe it should go to a clock driver. I don't really > > > > know. But this dts stuff "looks" right, so I'd rather leave it until we > > > > know that it will be useless. > > > > > > Ok! I understand your point. Because those are the last two in the > > > series, I > > > think there is no need to send v3 of this series without them. Greg, let > > > me know > > > if you prefer me to send v3 without last two patches. > > > > I'll just drop the last two from my queue, no need to resend. > > Greg, It seems last two patches were finally applied and they > shouldn't be. Let me know if you need > me to send patches to revert them. Ugh, my fault, let me go redo that branch and drop them, thanks for catching this... greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] Drivers: hv: vmbus: Add hooks for per-CPU IRQ
On Mon, Nov 26, 2018 at 08:56:50PM +, Michael Kelley wrote: > From: Greg KH Monday, November 26, 2018 11:57 AM > > > > > You created "null" hooks that do nothing, for no one in this patch > > > > series, why? > > > > > > > > > > hv_enable_vmbus_irq() and hv_disable_vmbus_irq() have non-null > > > implementations in the ARM64 code in patch 2 of this series. The > > > implementations are in the new file arch/arm64/hyperv/mshyperv.c. > > > Or am I misunderstanding your point? > > > > So you use a hook in an earlier patch and then add it in a later one? > > > > Shouldn't you do it the other way around? As it is, the earlier patch > > should not work properly, right? > > The earlier patch implements the hook on the ARM64 side but it is > unused -- it's not called. The later patch then calls it. Wouldn't the > other way around be backwards? Ah, it wasn't obvious that the previous patch added it at all, why not just make that addition part of this patch? > The general approach is for patches 1 and 2 of the series to provide > all the new code under arch/arm64 to enable Hyper-V. But the code > won't get called (or even built) with just these two patches because > CONFIG_HYPERV can't be selected. Patch 3 is separate because it > applies to architecture independent code and arch/x86 code -- I thought > there might be value in keeping the ARM64 and x86 patches distinct. > Patch 4 applies to architecture independent code, and enables the > ARM64 code in patches 1 and 2 to be compiled and run when > CONFIG_HYPERV is selected. > > If combining some of the patches in the series is a better approach, I'm > good with that. Ok, that makes more sense, if it is easier to get the ARM people to review this, that's fine. Doesn't seem like anyone did that yet :( sorry for the noise, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/16] staging: vchiq: dead code removal & misc fixes
On Mon, Nov 26, 2018 at 08:36:33PM +0100, Stefan Wahren wrote: > > Nicolas Saenz Julienne hat am 20. November 2018 um > > 15:53 geschrieben: > > > > > > Hi All, > > > > This series was written in parallel with reading and understanding the > > vchiq code. So excuse me for the lack of logic in the sequence of > > patches. > > > > The main focus was to delete as much code as possible, I've counted > > around 550 lines, which is not bad. Apart from that there are some > > patches enforcing proper kernel APIs usage. > > > > The only patch that really changes code is the > > vchiq_ioc_copy_element_data() rewrite. > > > > The last commit updates the TODO list with some of my observations, I > > realise some of the might be a little opinionated. If anything it's > > going to force a discussion on the topic, which is nice. > > > > It was developed on top of the latest linux-next, and was tested on a > > RPIv3B+ with audio, video and running vchiq_test. > > > > Regards, > > Nicolas > > > > RFC -> PATCH, as per Stefan's comments: > > - Remove semaphore initialization from remove_event_create() > > (commit 9) > > - Join all three semaphore to completion patches (commit 11) > > - Update probe/init commit message (commit 14) > > - Update TODO commit message and clean up (commit 16) > > - Fix spelling on some of the patches > > > > The whole series is > > Acked-by: Stefan Wahren > > Unfortunately patch 05 might not apply. I fixed it up. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[GIT PULL] Staging/IIO driver fixes for 4.20-rc5
The following changes since commit 651022382c7f8da46cb4872a545ee1da6d097d2a: Linux 4.20-rc1 (2018-11-04 15:37:52 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git tags/staging-4.20-rc5 for you to fetch changes up to c648284f6c9606f1854816086593eeae5556845a: Merge tag 'iio-fixes-for-4.20a' of git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio into staging-linus (2018-11-22 09:37:36 +0100) Staging and IIO driver fixes for 4.20-rc5 Here are some small IIO and Staging driver fixes for 4.20-rc5. Nothing major, the IIO fix ended up touching the HID drivers at the same time, but the HID maintainer acked it. The staging fixes are all minor patches for reported issues and regressions, full details are in the shortlog. All of these have been in linux-next for a while with no reported issues. Signed-off-by: Greg Kroah-Hartman Ben Wolsieffer (1): staging: vchiq_arm: fix compat VCHIQ_IOC_AWAIT_COMPLETION Christophe JAILLET (1): staging: rtl8723bs: Fix the return value in case of error in 'rtw_wx_read32()' Colin Ian King (3): drivers: staging: cedrus: find ctx before dereferencing it ctx staging: most: use format specifier "%s" in snprintf staging: mt7621-pinctrl: fix uninitialized variable ngroups Greg Kroah-Hartman (1): Merge tag 'iio-fixes-for-4.20a' of git://git.kernel.org/.../jic23/iio into staging-linus Hans de Goede (1): iio/hid-sensors: Fix IIO_CHAN_INFO_RAW returning wrong values for signed numbers Larry Finger (2): staging: rtl8723bs: Fix incorrect sense of ether_addr_equal staging: rtl8723bs: Add missing return for cfg80211_rtw_get_station Martin Kelly (1): iio:st_magn: Fix enable device after trigger Sergio Paracuellos (1): staging: mt7621-dma: fix potentially dereferencing uninitialized 'tx_desc' Spencer E. Olson (2): staging: comedi: ni_mio_common: scale ao INSN_CONFIG_GET_CMD_TIMING_CONSTRAINTS staging: comedi: clarify/unify macros for NI macro-defined terminals drivers/hid/hid-sensor-custom.c| 2 +- drivers/hid/hid-sensor-hub.c | 13 ++-- drivers/iio/accel/hid-sensor-accel-3d.c| 5 ++- drivers/iio/gyro/hid-sensor-gyro-3d.c | 5 ++- drivers/iio/humidity/hid-sensor-humidity.c | 3 +- drivers/iio/light/hid-sensor-als.c | 8 +++-- drivers/iio/light/hid-sensor-prox.c| 8 +++-- drivers/iio/magnetometer/hid-sensor-magn-3d.c | 8 +++-- drivers/iio/magnetometer/st_magn_buffer.c | 12 ++- drivers/iio/orientation/hid-sensor-incl-3d.c | 8 +++-- drivers/iio/pressure/hid-sensor-press.c| 8 +++-- drivers/iio/temperature/hid-sensor-temperature.c | 3 +- drivers/rtc/rtc-hid-sensor-time.c | 2 +- drivers/staging/comedi/comedi.h| 39 -- drivers/staging/comedi/drivers/ni_mio_common.c | 3 +- drivers/staging/media/sunxi/cedrus/cedrus.c| 22 ++-- drivers/staging/most/core.c| 2 +- drivers/staging/mt7621-dma/mtk-hsdma.c | 3 +- drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c| 2 +- drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c | 4 +-- drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 2 +- drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 2 +- .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 7 +++- include/linux/hid-sensor-hub.h | 4 ++- 24 files changed, 103 insertions(+), 72 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/29] staging: wilc1000: avoid deferring of cfg80211 operation callback
On Sun, Dec 02, 2018 at 06:02:13PM +, ajay.kat...@microchip.com wrote: > 6 files changed, 755 insertions(+), 2012 deletions(-) Nice code removal, patch series all now applied. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] binder: fix sparse warnings on locking context
On Mon, Dec 03, 2018 at 12:24:54PM -0800, Todd Kjos wrote: > Add __acquire()/__release() annnotations to fix warnings > in sparse context checking > > There is one case where the warning was due to a lack of > a "default:" case in a switch statement where a lock was > being released in each of the cases, so the default > case was added. > > Signed-off-by: Todd Kjos You sent out 4 patches here, as a series, but with no numbering so I don't know what order to put them in :( Can you resend them properly numbered so I have a chance to get it right? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] binder: fix kerneldoc header for struct binder_buffer
On Mon, Dec 03, 2018 at 12:24:55PM -0800, Todd Kjos wrote: > Fix the incomplete kerneldoc header for struct binder_buffer. > > Change-Id: If3ca10cf6d90f605a0c078e4cdce28f02a475877 No need for this here :) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] binder: fix use-after-free due to fdget() optimization
On Mon, Dec 03, 2018 at 12:24:57PM -0800, Todd Kjos wrote:
> 44d8047f1d87a ("binder: use standard functions to allocate fds")
> exposed a pre-existing issue in the binder driver.
>
> fdget() is used in ksys_ioctl() as a performance optimization.
> One of the rules associated with fdget() is that ksys_close() must
> not be called between the fdget() and the fdput(). There is a case
> where this requirement is not met in the binder driver (and possibly
> other drivers) which results in the reference count dropping to 0
> when the device is still in use. This can result in use-after-free
> or other issues.
>
> This was observed with the following sequence of events:
>
> Task A and task B are connected via binder; task A has /dev/binder open at
> file descriptor number X. Both tasks are single-threaded.
>
> 1. task B sends a binder message with a file descriptor array
>(BINDER_TYPE_FDA) containing one file descriptor to task A
> 2. task A reads the binder message with the translated file
>descriptor number Y
> 3. task A uses dup2(X, Y) to overwrite file descriptor Y with
>the /dev/binder file
> 4. task A unmaps the userspace binder memory mapping; the reference
>count on task A's /dev/binder is now 2
> 5. task A closes file descriptor X; the reference count on task
>A's /dev/binder is now 1
> 6. task A forks off a child, task C, duplicating the file descriptor
>table; the reference count on task A's /dev/binder is now 2
> 7. task A invokes the BC_FREE_BUFFER command on file descriptor X
>to release the incoming binder message
> 8. fdget() in ksys_ioctl() suppresses the reference count increment,
>since the file descriptor table is not shared
> 9. the BC_FREE_BUFFER handler removes the file descriptor table
>entry for X and decrements the reference count of task A's
>/dev/binder file to 1
> 10.task C calls close(X), which drops the reference count of
>task A's /dev/binder to 0 and frees it
> 11.task A continues processing of the ioctl and accesses some
>property of e.g. the binder_proc => KASAN-detectable UAF
>
> Fixed by using get_file() / fput() in binder_ioctl().
>
> Suggested-by: Jann Horn
> Signed-off-by: Todd Kjos
> Acked-by: Martijn Coenen
Shouldn't this go to 4.20-final? And have a stable@ tag?
And a "Fixes:" tag?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] binder: implement binderfs
On Tue, Dec 04, 2018 at 02:12:39PM +0100, Christian Brauner wrote:
> As discussed at Linux Plumbers Conference 2018 in Vancouver [1] this is the
> implementation of binderfs. If you want to skip reading and just see how it
> works, please go to [2].
First off, thanks for doing this so quickly. I think the overall idea
and implementation is great, I just have some minor issues with the user
api:
> /* binder-control */
> Each new binderfs instance comes with a binder-control device. No other
> devices will be present at first. The binder-control device can be used to
> dynamically allocate binder devices. All requests operate on the binderfs
> mount the binder-control device resides in:
> - BINDER_CTL_ADD
> Allocate a new binder device.
> Assuming a new instance of binderfs has been mounted at /dev/binderfs via
> mount -t binderfs binderfs /dev/binderfs. Then a request to create a new
> binder device can be made via:
>
> struct binderfs_device device = {0};
> int fd = open("/dev/binderfs/binder-control", O_RDWR);
> ioctl(fd, BINDER_CTL_ADD, &device);
>
> The struct binderfs_device will be used to return the major and minor
> number, as well as the index used as the new name for the device.
> Binderfs devices can simply be removed via unlink().
I think you should provide a name in the BINDER_CTL_ADD command. That
way you can easily emulate the existing binder queues, and it saves you
a create/rename sequence that you will be forced to do otherwise. Why
not do it just in a single command?
That way also you don't need to care about the major/minor number at
all. Userspace should never need to worry about that, use a name,
that's the best thing. Also, it allows you to drop the use of the idr,
making the kernel code simpler overall.
> /* Implementation details */
> - When binderfs is registered as a new filesystem it will dynamically
> allocate a new major number. The allocated major number will be returned
> in struct binderfs_device when a new binder device is allocated.
Why does userspace care about major/minor numbers at all? You should
just be able to deal with the binder "names", that's all that userspace
uses normally as you are not calling mknod() yourself.
> Minor numbers that have been given out are tracked in a global idr struct
> that is capped at BINDERFS_MAX_MINOR. The minor number tracker is
> protected by a global mutex. This is the only point of contention between
> binderfs mounts.
I doubt this will be any real contention given that setting up / tearing
down binder mounts is going to be rare, right? Well, hopefully, who
knows with some container systems...
> - The naming scheme for binder devices is binder%d. Each binderfs mount
> starts numbering of new binder devices at 0 up to n. The indeces used in
> constructing the name are tracked in a struct idr that is per-binderfs
> super block.
Again, let userspace pick the name, as you will have to rename it anyway
to get userspace to work properly with it.
I'll stop repeating myself now :)
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] binder: implement binderfs
On Wed, Dec 05, 2018 at 10:42:06PM +0100, Christian Brauner wrote:
> On Wed, Dec 05, 2018 at 09:01:45PM +0100, Greg KH wrote:
> > > /* binder-control */
> > > Each new binderfs instance comes with a binder-control device. No other
> > > devices will be present at first. The binder-control device can be used to
> > > dynamically allocate binder devices. All requests operate on the binderfs
> > > mount the binder-control device resides in:
> > > - BINDER_CTL_ADD
> > > Allocate a new binder device.
> > > Assuming a new instance of binderfs has been mounted at /dev/binderfs via
> > > mount -t binderfs binderfs /dev/binderfs. Then a request to create a new
> > > binder device can be made via:
> > >
> > > struct binderfs_device device = {0};
> > > int fd = open("/dev/binderfs/binder-control", O_RDWR);
> > > ioctl(fd, BINDER_CTL_ADD, &device);
> > >
> > > The struct binderfs_device will be used to return the major and minor
> > > number, as well as the index used as the new name for the device.
> > > Binderfs devices can simply be removed via unlink().
> >
> > I think you should provide a name in the BINDER_CTL_ADD command. That
> > way you can easily emulate the existing binder queues, and it saves you
> > a create/rename sequence that you will be forced to do otherwise. Why
> > not do it just in a single command?
>
> Sounds reasonable. How do you feel about capping the name length at 255
> bytes aka the standard Linux file name length (e.g. xfs, ext4 etc.)?
>
> #define BINDERFS_NAME_MAX 255
>
> struct binderfs_device {
> char name[BINDERFS_NAME_MAX + 1];
__u8 is the proper type to cross the user/kernel boundry :)
> __u32 major;
> __u32 minor;
> }
Yes, limiting it to 255 is fine with me.
> > That way also you don't need to care about the major/minor number at
> > all. Userspace should never need to worry about that, use a name,
> > that's the best thing. Also, it allows you to drop the use of the idr,
> > making the kernel code simpler overall.
> >
> > > /* Implementation details */
> > > - When binderfs is registered as a new filesystem it will dynamically
> > > allocate a new major number. The allocated major number will be returned
> > > in struct binderfs_device when a new binder device is allocated.
> >
> > Why does userspace care about major/minor numbers at all? You should
>
> Userspace cares for the sake of the devices cgroup which operates on
> device numnbers to restrict access to devices. Since binderfs doesn't
> have a static major number returning that information is helpful.
Ugh, ok, that makes sense. If we really want to make the kernel
interface simpler, drop the major/minor and then have userspace do the
stat(2) to see what the major/minor number they care about is.
But yeah, keeping it here makes everyone's life simpler, the kernel
already knows this, and it's trivial to pass it back to userspace this
way.
Care to make this change and resend?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] binder: fix sparse warnings on locking context
On Wed, Dec 05, 2018 at 03:19:24PM -0800, Todd Kjos wrote: > Add __acquire()/__release() annnotations to fix warnings > in sparse context checking > > There is one case where the warning was due to a lack of > a "default:" case in a switch statement where a lock was > being released in each of the cases, so the default > case was added. > > Signed-off-by: Todd Kjos > --- > v2: no change, just resubmitted as #1 of 3 patches instead of by itself I've already applied this one, right? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3] Add devicetree support for ad5933
On Sat, Dec 08, 2018 at 04:56:45PM -0200, Marcelo Schmitt wrote: > Parts of this work came from contributions of Alexandru Ardelean and > Dragos Bogdan, I and Gabriel would like to thank for the insights > provided by their previous patches. Maybe it would be the case to add > them as co-authors of this patch set. That's what the Co-developed-by: tag is for, please use it :) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[GIT PULL] Staging/IIO driver fixes for 4.20-rc6
The following changes since commit 2595646791c319cadfdbf271563aac97d0843dc7: Linux 4.20-rc5 (2018-12-02 15:07:55 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git tags/staging-4.20-rc6 for you to fetch changes up to 87e4a5405f087427fbf8b437d2796283dce2b38f: Revert commit ef9209b642f "staging: rtl8723bs: Fix indenting errors and an off-by-one mistake in core/rtw_mlme_ext.c" (2018-12-05 09:56:09 +0100) Staging fixes for 4.20-rc6 Here are two staging driver bugfixes for 4.20-rc6. One is a revert of a previously incorrect patch that was merged a while ago, and the other resolves a possible buffer overrun that was found by code inspection. Both of these have been in the linux-next tree with no reported issues. Signed-off-by: Greg Kroah-Hartman Young Xiao (2): staging: rtl8712: Fix possible buffer overrun Revert commit ef9209b642f "staging: rtl8723bs: Fix indenting errors and an off-by-one mistake in core/rtw_mlme_ext.c" drivers/staging/rtl8712/mlme_linux.c | 2 +- drivers/staging/rtl8712/rtl871x_mlme.c| 2 +- drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] MAINTAINERS: Patch monkey for the Hyper-V code
On Tue, Dec 11, 2018 at 12:09:49PM -0500, Sasha Levin wrote: > Now the Hyper-V code has it's own monkey on a tree! > > Make it easier to manage patch flow to upper level maintainers. > > Acked-by: Haiyang Zhang > Acked-by: K. Y. Srinivasan > Signed-off-by: Sasha Levin > --- > MAINTAINERS | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 8119141a926f..553b1ed1d01f 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6901,8 +6901,10 @@ Hyper-V CORE AND DRIVERS > M: "K. Y. Srinivasan" > M: Haiyang Zhang > M: Stephen Hemminger > +M: Sasha Levin > +T: git git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git A "project" git tree, that's odd for such a small subsystem... > L: de...@linuxdriverproject.org > -S: Maintained > +S: Supported Yeah! Finally someone is getting paid to do this :) greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC] staging: comedi: fix coding style issues
On Thu, Feb 04, 2021 at 05:43:37PM +0530, Ayush wrote:
> fix IF_0 and IF_1 warnings by checkpatch.pl
What is that?
> Signed-off-by: Ayush
> ---
> compile tested only.
When you send something with "RFC" that means you do not think it is
good enough to be merged, and so I will just delete it. Also, you did
not ask a question that you want to be answered, so why is this RFC?
>
> drivers/staging/comedi/drivers/dt2801.c | 29 -
> 1 file changed, 29 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/dt2801.c
> b/drivers/staging/comedi/drivers/dt2801.c
> index 0d571d817b4e..bb01416084e4 100644
> --- a/drivers/staging/comedi/drivers/dt2801.c
> +++ b/drivers/staging/comedi/drivers/dt2801.c
> @@ -87,17 +87,6 @@
> #define DT2801_STATUS1
> #define DT2801_CMD 1
>
> -#if 0
> -/* ignore 'defined but not used' warning */
> -static const struct comedi_lrange range_dt2801_ai_pgh_bipolar = {
> - 4, {
> - BIP_RANGE(10),
> - BIP_RANGE(5),
> - BIP_RANGE(2.5),
> - BIP_RANGE(1.25)
> - }
> -};
> -#endif
Please leave these alone, it's good documentation.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: atomisp: fix coding style issues in timer.c
On Sun, Jan 31, 2021 at 11:55:29PM +0530, Ayush wrote: > - Fix unneeded brace in if condition(also, brace was on next line). > - Fix leading space warning before struct ia_css_clock_tick *curr_ts. > > compile tested only (on next-20210129) > > Signed-off-by: Ayush > --- > .../staging/media/atomisp/pci/runtime/timer/src/timer.c| 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - Your patch did many different things all at once, making it difficult to review. All Linux kernel patches need to only do one thing at a time. If you need to do multiple things (such as clean up all coding style issues in a file/driver), do it in a sequence of patches, each one doing only one thing. This will make it easier to review the patches to ensure that they are correct, and to help alleviate any merge issues that larger patches can cause. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: fbtft replaced udelay with usleep_range
On Fri, Feb 05, 2021 at 02:41:13PM +0530, Mayank Suman wrote: > According to Documentation/timers/timers-howto.rst, usleep_range is > preffered over udelay for >=10us delay. > > Signed-off-by: Mayank Suman ALWAYS test build your patches before sending them out to the world for review. You don't want to make maintainers grumpy by breaking the tree with changes that can not compile, right? :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] driver core: Use subdir-ccflags-* to inherit debug flag
On Fri, Feb 05, 2021 at 05:44:12PM +0800, Yicong Yang wrote: > From: Junhao He > > Use subdir-ccflags-* instead of ccflags-* to inherit the debug > settings from Kconfig when traversing subdirectories. That says what you do, but not _why_ you are doing it. What does this offer in benefit of the existing way? What is it fixing? Why do this "churn"? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] staging: comedi: Use subdir-ccflags-* to inherit debug flag
On Fri, Feb 05, 2021 at 05:44:15PM +0800, Yicong Yang wrote: > From: Junhao He > > Use subdir-ccflags-* instead of ccflags-* to inherit the debug > settings from Kconfig when traversing subdirectories. Again, explain _why_. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/SubmittingPatches for what a proper changelog should look like. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/7] seqnum_ops: Introduce Sequence Number Ops
On Wed, Feb 03, 2021 at 11:11:57AM -0700, Shuah Khan wrote:
> +static inline u32 seqnum32_inc(struct seqnum32 *seq)
> +{
> + atomic_t val = ATOMIC_INIT(seq->seqnum);
> +
> + seq->seqnum = (u32) atomic_inc_return(&val);
> + if (seq->seqnum >= UINT_MAX)
> + pr_info("Sequence Number overflow %u detected\n",
> + seq->seqnum);
> + return seq->seqnum;
As Peter points out, this is doing doing what you think it is doing :(
Why do you not just have seq->seqnum be a real atomic variable? Trying
to switch to/from one like this does not work as there is no
"atomic-ness" happening here at all.
Oh, and checkpatch should have complained about the extra ' ' in your
cast :)
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: emxx_udc: Fix incorrectly defined global
On Sun, Feb 07, 2021 at 05:30:31AM +0530, Kumar Kartikeya Dwivedi wrote: > The global gpio_desc pointer and int were defined in the header, > instead put the definitions in the translation unit and add an extern > declaration for consumers of the header (currently only one, which is > perhaps why the linker didn't complain about symbol collisions). > > This fixes sparse related warnings for this driver: > drivers/staging/emxx_udc/emxx_udc.c: note: in included file: > drivers/staging/emxx_udc/emxx_udc.h:23:18: warning: symbol 'vbus_gpio' was > not declared. Should it be static? > drivers/staging/emxx_udc/emxx_udc.h:24:5: warning: symbol 'vbus_irq' was not > declared. Should it be static? > > Signed-off-by: Kumar Kartikeya Dwivedi > --- > drivers/staging/emxx_udc/emxx_udc.c | 3 +++ > drivers/staging/emxx_udc/emxx_udc.h | 4 ++-- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/emxx_udc/emxx_udc.c > b/drivers/staging/emxx_udc/emxx_udc.c > index a30b4f5b1..6983c3e31 100644 > --- a/drivers/staging/emxx_udc/emxx_udc.c > +++ b/drivers/staging/emxx_udc/emxx_udc.c > @@ -34,6 +34,9 @@ > #define DRIVER_DESC "EMXX UDC driver" > #define DMA_ADDR_INVALID(~(dma_addr_t)0) > > +struct gpio_desc *vbus_gpio; > +int vbus_irq; A tiny driver can not create global variables with generic names like this, sorry. That will polute the global namespace. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: emxx_udc: Fix incorrectly defined global
On Sun, Feb 07, 2021 at 01:08:27PM +0530, Kumar Kartikeya Dwivedi wrote: > On Sun, Feb 07, 2021 at 12:04:41PM IST, Stephen Rothwell wrote: > > > > Given that drivers/staging/emxx_udc/emxx_udc.h is only included by > > drivers/staging/emxx_udc/emxx_udc.c, shouldn't these variables just be > > declared static in emxx_udc.c and removed from emxx_udc.h? > > > > Either would be correct. I went this way because it was originally trying to > (incorrectly) define a global variable instead. I guess they can be static now > and when more users are added, the linkage can be adjusted as needed. > > Here's another version of the patch: Please resend in the proper format that a second version of a patch should be in (the documentation describes how to do this.) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gasket: Align code to match with open parenthesis and fix the lines ending with open parenthesis
On Sun, Feb 07, 2021 at 01:11:36PM +0530, Mahak gupta wrote:
> This patch fixes warnings of checkpatch.pl. According to the coding style
> of linux, code should be aligned properly to match with open parenthesis
> and lines should not end with open parenthesis.
>
> Signed-off-by: mhk19
> ---
> drivers/staging/gasket/gasket_ioctl.c | 18 --
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/gasket/gasket_ioctl.c
> b/drivers/staging/gasket/gasket_ioctl.c
> index e3047d36d8db..a966231bad42 100644
> --- a/drivers/staging/gasket/gasket_ioctl.c
> +++ b/drivers/staging/gasket/gasket_ioctl.c
> @@ -40,7 +40,7 @@ static int gasket_set_event_fd(struct gasket_dev
> *gasket_dev,
>
> /* Read the size of the page table. */
> static int gasket_read_page_table_size(struct gasket_dev *gasket_dev,
> - struct gasket_page_table_ioctl __user *argp)
> + struct gasket_page_table_ioctl
> __user *argp)
> {
> int ret = 0;
> struct gasket_page_table_ioctl ibuf;
> @@ -51,8 +51,7 @@ static int gasket_read_page_table_size(struct gasket_dev
> *gasket_dev,
> if (ibuf.page_table_index >= gasket_dev->num_page_tables)
> return -EFAULT;
>
> - ibuf.size = gasket_page_table_num_entries(
> - gasket_dev->page_table[ibuf.page_table_index]);
> + ibuf.size =
> gasket_page_table_num_entries(gasket_dev->page_table[ibuf.page_table_index]);
>
> trace_gasket_ioctl_page_table_data(ibuf.page_table_index, ibuf.size,
>ibuf.host_address,
> @@ -66,7 +65,7 @@ static int gasket_read_page_table_size(struct gasket_dev
> *gasket_dev,
>
> /* Read the size of the simple page table. */
> static int gasket_read_simple_page_table_size(struct gasket_dev
> *gasket_dev,
> - struct gasket_page_table_ioctl __user *argp)
> + struct
> gasket_page_table_ioctl __user *argp)
> {
> int ret = 0;
> struct gasket_page_table_ioctl ibuf;
> @@ -92,7 +91,7 @@ static int gasket_read_simple_page_table_size(struct
> gasket_dev *gasket_dev,
>
> /* Set the boundary between the simple and extended page tables. */
> static int gasket_partition_page_table(struct gasket_dev *gasket_dev,
> - struct gasket_page_table_ioctl __user *argp)
> + struct gasket_page_table_ioctl
> __user *argp)
> {
> int ret;
> struct gasket_page_table_ioctl ibuf;
> @@ -107,8 +106,8 @@ static int gasket_partition_page_table(struct
> gasket_dev *gasket_dev,
>
> if (ibuf.page_table_index >= gasket_dev->num_page_tables)
> return -EFAULT;
> - max_page_table_size = gasket_page_table_max_size(
> - gasket_dev->page_table[ibuf.page_table_index]);
> + max_page_table_size = gasket_page_table_max_size
> + (gasket_dev->page_table[ibuf.page_table_index]);
>
> if (ibuf.size > max_page_table_size) {
> dev_dbg(gasket_dev->dev,
> @@ -119,8 +118,7 @@ static int gasket_partition_page_table(struct
> gasket_dev *gasket_dev,
>
> mutex_lock(&gasket_dev->mutex);
>
> - ret = gasket_page_table_partition(
> - gasket_dev->page_table[ibuf.page_table_index], ibuf.size);
> + ret =
> gasket_page_table_partition(gasket_dev->page_table[ibuf.page_table_index],
> ibuf.size);
> mutex_unlock(&gasket_dev->mutex);
>
> return ret;
> @@ -183,7 +181,7 @@ static int gasket_unmap_buffers(struct gasket_dev
> *gasket_dev,
> * corresponding memory.
> */
> static int gasket_config_coherent_allocator(struct gasket_dev *gasket_dev,
> - struct gasket_coherent_alloc_config_ioctl __user *argp)
> + struct
> gasket_coherent_alloc_config_ioctl __user *argp)
> {
> int ret;
> struct gasket_coherent_alloc_config_ioctl ibuf;
> --
> 2.17.1
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- Your patch is malformed (tabs converted to spaces, linewrapped, etc.)
and can not be applied. Please read the file,
Documentation/email-clients.txt in order to fix this.
- You sent multiple patches, yet no indication of which ones should be
applied in which order. Greg could just guess, but if you are
receiving this email, he guessed wrong and the patches didn't apply.
Please read the section entitled "The canonical patch format" in the
kernel file,
Re: [PATCH v2] staging: emxx_udc: Make incorrectly defined global static
On Sun, Feb 07, 2021 at 02:16:58PM +0530, Kumar Kartikeya Dwivedi wrote: > The global gpio_desc pointer and int vbus_irq were defined in the header, > instead put the definitions in the translation unit and make them static as > there's only a single consumer, and these symbols shouldn't pollute the > global namespace. > > This fixes the following sparse warnings for this driver: > drivers/staging/emxx_udc/emxx_udc.c: note: in included file: > drivers/staging/emxx_udc/emxx_udc.h:23:18: warning: symbol 'vbus_gpio' was not > declared. Should it be static? drivers/staging/emxx_udc/emxx_udc.h:24:5: > warning: symbol 'vbus_irq' was not declared. Should it be static? > > Signed-off-by: Kumar Kartikeya Dwivedi > --- > drivers/staging/emxx_udc/emxx_udc.c | 3 +++ > drivers/staging/emxx_udc/emxx_udc.h | 2 -- > 2 files changed, 3 insertions(+), 2 deletions(-) Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - This looks like a new version of a previously submitted patch, but you did not list below the --- line any changes from the previous version. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/SubmittingPatches for what needs to be done here to properly describe this. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: emxx_udc: Make incorrectly defined global static
On Sun, Feb 07, 2021 at 02:29:12PM +0530, Kumar Kartikeya Dwivedi wrote: > The global gpio_desc pointer and int vbus_irq were defined in the header, > instead put the definitions in the translation unit and make them static as > there's only a single consumer, and these symbols shouldn't pollute the > global namespace. > > This fixes the following sparse warnings for this driver: > drivers/staging/emxx_udc/emxx_udc.c: note: in included file: > drivers/staging/emxx_udc/emxx_udc.h:23:18: warning: symbol 'vbus_gpio' was not > declared. Should it be static? drivers/staging/emxx_udc/emxx_udc.h:24:5: > warning: symbol 'vbus_irq' was not declared. Should it be static? > > Signed-off-by: Kumar Kartikeya Dwivedi > --- > Changes in v1: > Switch to variable with static linkage instead of extern > Changes in v2: > Resend a versioned patch > Changes in v3: > Include version changelog below the marker Much better, thanks, now queued up. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gasket: fix indentation and lines ending with open parenthesis
On Sun, Feb 07, 2021 at 07:39:28PM +0530, Mahak Gupta wrote:
> This patch fixes warnings of 'checkpatch.pl'. According to
> Linux coding guidelines, code should be aligned properly to
> match with open parenthesis and lines should not end with
> open parenthesis.
>
> Signed-off-by: Mahak Gupta
> ---
> drivers/staging/gasket/gasket_ioctl.c | 18 --
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/gasket/gasket_ioctl.c
> b/drivers/staging/gasket/gasket_ioctl.c
> index e3047d36d8db..a966231bad42 100644
> --- a/drivers/staging/gasket/gasket_ioctl.c
> +++ b/drivers/staging/gasket/gasket_ioctl.c
> @@ -40,7 +40,7 @@ static int gasket_set_event_fd(struct gasket_dev
> *gasket_dev,
>
> /* Read the size of the page table. */
> static int gasket_read_page_table_size(struct gasket_dev *gasket_dev,
> - struct gasket_page_table_ioctl __user *argp)
> +struct gasket_page_table_ioctl __user
> *argp)
> {
> int ret = 0;
> struct gasket_page_table_ioctl ibuf;
> @@ -51,8 +51,7 @@ static int gasket_read_page_table_size(struct gasket_dev
> *gasket_dev,
> if (ibuf.page_table_index >= gasket_dev->num_page_tables)
> return -EFAULT;
>
> - ibuf.size = gasket_page_table_num_entries(
> - gasket_dev->page_table[ibuf.page_table_index]);
> + ibuf.size =
> gasket_page_table_num_entries(gasket_dev->page_table[ibuf.page_table_index]);
>
> trace_gasket_ioctl_page_table_data(ibuf.page_table_index, ibuf.size,
> ibuf.host_address,
> @@ -66,7 +65,7 @@ static int gasket_read_page_table_size(struct gasket_dev
> *gasket_dev,
>
> /* Read the size of the simple page table. */
> static int gasket_read_simple_page_table_size(struct gasket_dev *gasket_dev,
> - struct gasket_page_table_ioctl __user *argp)
> + struct gasket_page_table_ioctl
> __user *argp)
> {
> int ret = 0;
> struct gasket_page_table_ioctl ibuf;
> @@ -92,7 +91,7 @@ static int gasket_read_simple_page_table_size(struct
> gasket_dev *gasket_dev,
>
> /* Set the boundary between the simple and extended page tables. */
> static int gasket_partition_page_table(struct gasket_dev *gasket_dev,
> - struct gasket_page_table_ioctl __user *argp)
> +struct gasket_page_table_ioctl __user
> *argp)
> {
> int ret;
> struct gasket_page_table_ioctl ibuf;
> @@ -107,8 +106,8 @@ static int gasket_partition_page_table(struct gasket_dev
> *gasket_dev,
>
> if (ibuf.page_table_index >= gasket_dev->num_page_tables)
> return -EFAULT;
> - max_page_table_size = gasket_page_table_max_size(
> - gasket_dev->page_table[ibuf.page_table_index]);
> + max_page_table_size = gasket_page_table_max_size
> + (gasket_dev->page_table[ibuf.page_table_index]);
>
> if (ibuf.size > max_page_table_size) {
> dev_dbg(gasket_dev->dev,
> @@ -119,8 +118,7 @@ static int gasket_partition_page_table(struct gasket_dev
> *gasket_dev,
>
> mutex_lock(&gasket_dev->mutex);
>
> - ret = gasket_page_table_partition(
> - gasket_dev->page_table[ibuf.page_table_index], ibuf.size);
> + ret =
> gasket_page_table_partition(gasket_dev->page_table[ibuf.page_table_index],
> ibuf.size);
> mutex_unlock(&gasket_dev->mutex);
>
> return ret;
> @@ -183,7 +181,7 @@ static int gasket_unmap_buffers(struct gasket_dev
> *gasket_dev,
> * corresponding memory.
> */
> static int gasket_config_coherent_allocator(struct gasket_dev *gasket_dev,
> - struct gasket_coherent_alloc_config_ioctl __user *argp)
> + struct
> gasket_coherent_alloc_config_ioctl __user *argp)
> {
> int ret;
> struct gasket_coherent_alloc_config_ioctl ibuf;
> --
> 2.17.1
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/SubmittingPatches for what needs to be done
here to properly describe this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out fro
Re: [PATCH] staging: octeon: convert all uses of strlcpy to strscpy in ethernet-mdio.c
On Sun, Feb 07, 2021 at 02:48:04PM +, Phillip Potter wrote: > Convert three calls to strlcpy inside the cvm_oct_get_drvinfo function > to strscpy calls. Fixes a style warning. Is it really safe to do this type of conversion here? If so, you need to provide evidence of it in the changelog, otherwise we could just do a search/replace across the whole kernel and be done with it :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: octeon: convert all uses of strlcpy to strscpy in ethernet-mdio.c
On Sun, Feb 07, 2021 at 03:03:02PM +, Phillip Potter wrote:
> Convert three calls to strlcpy inside the cvm_oct_get_drvinfo function
> to strscpy calls. As return values were not checked for these three
> calls before, change should be safe as functionality is equivalent.
>
> Signed-off-by: Phillip Potter
> ---
> drivers/staging/octeon/ethernet-mdio.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/octeon/ethernet-mdio.c
> b/drivers/staging/octeon/ethernet-mdio.c
> index b0fd083a5bf2..b3049108edc4 100644
> --- a/drivers/staging/octeon/ethernet-mdio.c
> +++ b/drivers/staging/octeon/ethernet-mdio.c
> @@ -21,9 +21,9 @@
> static void cvm_oct_get_drvinfo(struct net_device *dev,
> struct ethtool_drvinfo *info)
> {
> - strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
> - strlcpy(info->version, UTS_RELEASE, sizeof(info->version));
> - strlcpy(info->bus_info, "Builtin", sizeof(info->bus_info));
> + strscpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
> + strscpy(info->version, UTS_RELEASE, sizeof(info->version));
> + strscpy(info->bus_info, "Builtin", sizeof(info->bus_info));
> }
>
> static int cvm_oct_nway_reset(struct net_device *dev)
> --
> 2.29.2
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/SubmittingPatches for what needs to be done
here to properly describe this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: octeon: convert all uses of strlcpy to strscpy in ethernet-mdio.c
On Sun, Feb 07, 2021 at 03:13:20PM +, Phillip Potter wrote:
> Convert three calls to strlcpy inside the cvm_oct_get_drvinfo function
> to strscpy calls. As return values were not checked for these three
> calls before, change should be safe as functionality is equivalent.
>
> Signed-off-by: Phillip Potter
> ---
>
> v2: Modified changelog to take account of feedback from Greg KH.
>
> drivers/staging/octeon/ethernet-mdio.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/octeon/ethernet-mdio.c
> b/drivers/staging/octeon/ethernet-mdio.c
> index b0fd083a5bf2..b3049108edc4 100644
> --- a/drivers/staging/octeon/ethernet-mdio.c
> +++ b/drivers/staging/octeon/ethernet-mdio.c
> @@ -21,9 +21,9 @@
> static void cvm_oct_get_drvinfo(struct net_device *dev,
> struct ethtool_drvinfo *info)
> {
> - strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
> - strlcpy(info->version, UTS_RELEASE, sizeof(info->version));
> - strlcpy(info->bus_info, "Builtin", sizeof(info->bus_info));
> + strscpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
> + strscpy(info->version, UTS_RELEASE, sizeof(info->version));
> + strscpy(info->bus_info, "Builtin", sizeof(info->bus_info));
> }
>
> static int cvm_oct_nway_reset(struct net_device *dev)
Sorry, this does not apply to my tree, someone already did this
conversion before you :(
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [96e8740] [PATCH 2/2] Staging: wimax: i2400m: some readability improvements.
On Sun, Feb 07, 2021 at 10:11:24PM +0300, dev.dra...@bk.ru wrote:
> From: Dmitrii Wolf
>
> Hello, developers!
> Sorry for the late answer. As you know - i am a newbie and it is my first
> kernel patch.
> After reading kernelnewbies.or, ./Documentation/process/ files and viewing
> FOSDEM's videpo
> "Write and Submit your first Linux kernel Patch", i took a decision to send
> you some
> changes. I understand that it is annoying to get this "style fixing"
> patches. So, the
> Joe Perches's idea to improve code readability was implemented in second
> patch. Also,
> some new readability improvements added to it.
> Thanks in advance!
>
> Signed-off-by: Dmitrii Wolf
> ---
> drivers/staging/wimax/i2400m/netdev.c | 8
> drivers/staging/wimax/i2400m/rx.c | 25 +
> 2 files changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/staging/wimax/i2400m/netdev.c
> b/drivers/staging/wimax/i2400m/netdev.c
> index 0895a2e441d3..5f79ccc87656 100644
> --- a/drivers/staging/wimax/i2400m/netdev.c
> +++ b/drivers/staging/wimax/i2400m/netdev.c
> @@ -366,13 +366,13 @@ netdev_tx_t i2400m_hard_start_xmit(struct sk_buff *skb,
> result = i2400m_net_wake_tx(i2400m, net_dev, skb);
> else
> result = i2400m_net_tx(i2400m, net_dev, skb);
> - if (result < 0) {
> -drop:
> - net_dev->stats.tx_dropped++;
> - } else {
> + if (result >= 0) {
> net_dev->stats.tx_packets++;
> net_dev->stats.tx_bytes += skb->len;
> }
> +drop:
> + net_dev->stats.tx_dropped++;
> +
> dev_kfree_skb(skb);
> d_fnend(3, dev, "(skb %p net_dev %p) = %d\n", skb, net_dev, result);
> return NETDEV_TX_OK;
> diff --git a/drivers/staging/wimax/i2400m/rx.c
> b/drivers/staging/wimax/i2400m/rx.c
> index 807bd3db69e9..fdc5da409683 100644
> --- a/drivers/staging/wimax/i2400m/rx.c
> +++ b/drivers/staging/wimax/i2400m/rx.c
> @@ -194,8 +194,8 @@ void i2400m_report_hook_work(struct work_struct *ws)
> spin_unlock_irqrestore(&i2400m->rx_lock, flags);
> if (list_empty(&list))
> break;
> - else
> - d_printf(1, dev, "processing queued reports\n");
> +
> + d_printf(1, dev, "processing queued reports\n");
> list_for_each_entry_safe(args, args_next, &list, list_node) {
> d_printf(2, dev, "processing queued report %p\n", args);
> i2400m_report_hook(i2400m, args->l3l4_hdr, args->size);
> @@ -756,16 +756,15 @@ unsigned __i2400m_roq_update_ws(struct i2400m *i2400m,
> struct i2400m_roq *roq,
> roq_data_itr = (struct i2400m_roq_data *) &skb_itr->cb;
> nsn_itr = __i2400m_roq_nsn(roq, roq_data_itr->sn);
> /* NSN bounds assumed correct (checked when it was queued) */
> - if (nsn_itr < new_nws) {
> - d_printf(2, dev, "ERX: roq %p - release skb %p "
> - "(nsn %u/%u new nws %u)\n",
> - roq, skb_itr, nsn_itr, roq_data_itr->sn,
> - new_nws);
> - __skb_unlink(skb_itr, &roq->queue);
> - i2400m_net_erx(i2400m, skb_itr, roq_data_itr->cs);
> - } else {
> + if (nsn_itr >= new_nws) {
> break; /* rest of packets all nsn_itr > nws */
> }
> + d_printf(2, dev, "ERX: roq %p - release skb %p "
> + "(nsn %u/%u new nws %u)\n",
> + roq, skb_itr, nsn_itr, roq_data_itr->sn,
> + new_nws);
> + __skb_unlink(skb_itr, &roq->queue);
> + i2400m_net_erx(i2400m, skb_itr, roq_data_itr->cs);
> }
> roq->ws = sn;
> return new_nws;
> @@ -904,8 +903,9 @@ void i2400m_roq_queue_update_ws(struct i2400m *i2400m,
> struct i2400m_roq *roq,
> struct i2400m_roq_data *roq_data;
> roq_data = (struct i2400m_roq_data *) &skb->cb;
> i2400m_net_erx(i2400m, skb, roq_data->cs);
> - } else
> + } else {
> __i2400m_roq_queue(i2400m, roq, skb, sn, nsn);
> + }
>
> __i2400m_roq_update_ws(i2400m, roq, sn + 1);
> i2400m_roq_log_add(i2400m, roq, I2400M_RO_TYPE_PACKET_WS,
> @@ -1321,9 +1321,10 @@ void i2400m_unknown_barker(struct i2400m *i2400m,
> 8, 4, buf, 64, 0);
> printk(KERN_ERR "%s... (only first 64 bytes "
> "dumped)\n", prefix);
> - } else
> + } else {
> print_hex_dump(KERN_ERR, prefix, DUMP_PREFIX_OFFSET,
> 8, 4, buf, size, 0);
> + }
> }
> EXPORT_SYMBOL(i2400m_unknown_barker);
>
> --
> 2.25.1
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these c
Re: [PATCH 1/4] driver core: Use subdir-ccflags-* to inherit debug flag
On Mon, Feb 08, 2021 at 06:44:52PM +0800, Yicong Yang wrote: > Hi Greg, > > On 2021/2/5 17:53, Greg KH wrote: > > On Fri, Feb 05, 2021 at 05:44:12PM +0800, Yicong Yang wrote: > >> From: Junhao He > >> > >> Use subdir-ccflags-* instead of ccflags-* to inherit the debug > >> settings from Kconfig when traversing subdirectories. > > > > That says what you do, but not _why_ you are doing it. > > i'll illustrate the reason and reword the commit. > > > > > What does this offer in benefit of the existing way? What is it fixing? > > Why do this "churn"? > > currently we have added ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG in the > Makefile > of driver/base and driver/base/power, but not in the subdirectory > driver/base/firmware_loader. we cannot turn the debug on for subdirectory > firmware_loader if we config DEBUG_DRIVER and there is no kconfig option > for the it. Is that necessary? Does that directory need it? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH]: checkpatch: Fixed styling issue
On Mon, Feb 08, 2021 at 06:36:14PM +0530, Mukul Mehar wrote:
> >From 29bcaf0066003983da29b1e026b985c0727b091a Mon Sep 17 00:00:00 2001
> From: Mukul Mehar
> Date: Mon, 8 Feb 2021 01:03:06 +0530
> Subject: [PATCH] Drivers: staging: most: sound: Fixed style issue.
> Signed-off-by: Mukul Mehar
>
>
> This patch fixes a warning, of the line ending with a '(',
> generated by checkpatch.pl.
> This is my first patch.
> ---
> drivers/staging/most/sound/sound.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/most/sound/sound.c
> b/drivers/staging/most/sound/sound.c
> index 3a1a59058042..4dd1bf95d1ce 100644
> --- a/drivers/staging/most/sound/sound.c
> +++ b/drivers/staging/most/sound/sound.c
> @@ -228,12 +228,12 @@ static int playback_thread(void *data)
> struct mbo *mbo = NULL;
> bool period_elapsed = false;
>
> - wait_event_interruptible(
> - channel->playback_waitq,
> - kthread_should_stop() ||
> - (channel->is_stream_running &&
> - (mbo = most_get_mbo(channel->iface, channel->id,
> - ∁
> + wait_event_interruptible(channel->playback_waitq,
> + kthread_should_stop() ||
> + (channel->is_stream_running &&
> + (mbo = most_get_mbo(channel->iface,
> + channel->id,
> + ∁
> if (!mbo)
> continue;
>
> --
> 2.25.1
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- Your patch was attached, please place it inline so that it can be
applied directly from the email message itself.
- Your patch does not have a Signed-off-by: line. Please read the
kernel file, Documentation/SubmittingPatches and resend it after
adding that line. Note, the line needs to be in the body of the
email, before the patch, not at the bottom of the patch or in the
email signature.
- You did not specify a description of why the patch is needed, or
possibly, any description at all, in the email body. Please read the
section entitled "The canonical patch format" in the kernel file,
Documentation/SubmittingPatches for what is needed in order to
properly describe the change.
- You did not write a descriptive Subject: for the patch, allowing Greg,
and everyone else, to know what this patch is all about. Please read
the section entitled "The canonical patch format" in the kernel file,
Documentation/SubmittingPatches for what a proper Subject: line should
look like.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH]: checkpatch: Fixed styling issue
On Mon, Feb 08, 2021 at 11:58:02PM +0530, Mukul Mehar wrote:
> >From 29bcaf0066003983da29b1e026b985c0727b091a Mon Sep 17 00:00:00 2001
> From: Mukul Mehar
> Date: Mon, 8 Feb 2021 01:03:06 +0530
> Subject: [PATCH] Drivers: staging: most: sound: Fixed style issue.
>
> This patch fixes a warning, of the line ending with a '(',
> generated by checkpatch.pl.
> This is my first patch.
>
> Signed-off-by: Mukul Mehar
> ---
> drivers/staging/most/sound/sound.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
You ignored everything my bot sent last time, why, do you somehow think
it was not correct? {hint, it was...}
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
