Re: [Drbd-dev] [PATCH] drbd: Fix a use after free in get_initial_state

[Christoph Böhmwalder]

On 4/1/21 1:57 PM, Lv Yunlong wrote:

In get_initial_state, it calls notify_initial_state_done(skb,..) if
cb->args[5]==1. I see that if genlmsg_put() failed in
notify_initial_state_done(), the skb will be freed by nlmsg_free(skb).
Then get_initial_state will goto out and the freed skb will be used by
return value skb->len.

My patch lets skb_len = skb->len and return the skb_len to avoid the uaf.

Fixes: a29728463b254 ("drbd: Backport the "events2" command")
Signed-off-by: Lv Yunlong 
---
  drivers/block/drbd/drbd_nl.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index bf7de4c7b96c..474f84675d0a 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -4905,6 +4905,7 @@ static int get_initial_state(struct sk_buff *skb, struct 
netlink_callback *cb)
struct drbd_state_change *state_change = (struct drbd_state_change 
*)cb->args[0];
unsigned int seq = cb->args[2];
unsigned int n;
+   unsigned int skb_len = skb->len;
enum drbd_notification_type flags = 0;
  
  	/* There is no need for taking notification_mutex here: it doesn't

@@ -4915,7 +4916,7 @@ static int get_initial_state(struct sk_buff *skb, struct 
netlink_callback *cb)
cb->args[5]--;
if (cb->args[5] == 1) {
notify_initial_state_done(skb, seq);
-   goto out;
+   return skb_len;
}
n = cb->args[4]++;
if (cb->args[4] < cb->args[3])



Thanks for the patch!

I think the problem goes even further: skb can also be freed in the 
notify_*_state_change -> notify_*_state calls below.


Also, at the point where we save skb->len into skb_len, skb is not 
initialized yet. Maybe it makes more sense to not return a length in the 
first place here, but an error code instead.


--
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage

Re: [PATCH] crypto: expose needs_key in procfs

[Christoph Böhmwalder]

On 01.03.21 19:47, Eric Biggers wrote:

On Mon, Mar 01, 2021 at 05:59:17PM +0100, Christoph Böhmwalder wrote:

Currently, it is not apparent for userspace users which hash algorithms
require a key and which don't. We have /proc/crypto, so add a field
with this information there.

Signed-off-by: Christoph Böhmwalder 

---
  crypto/shash.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/crypto/shash.c b/crypto/shash.c
index 2e3433ad9762..d3127a0618f2 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -477,6 +477,9 @@ static void crypto_shash_show(struct seq_file *m, struct 
crypto_alg *alg)
seq_printf(m, "type : shash\n");
seq_printf(m, "blocksize: %u\n", alg->cra_blocksize);
seq_printf(m, "digestsize   : %u\n", salg->digestsize);
+   seq_printf(m, "needs key: %s\n",
+  crypto_shash_alg_needs_key(salg) ?
+  "yes" : "no");
  }
  


Do you have a specific use case in mind for this information?  Normally, users
should already know which algorithm they want to use (or set of algorithms they
might want to use).


I have a pretty specific use case in mind, yes. For DRBD, we use crypto 
algorithms for peer authentication and for the online-verify mechanism 
(to verify data integrity). The peer authentication algos require a 
shared secret (HMAC), while the verify algorithms are just hash 
functions without keys (we don't configure a shared secret here, so 
these must explicitly be "keyless").


Now, we also have a solution which sits on top of DRBD (LINSTOR), which 
resides purely in userspace. We recently implemented a feature where 
LINSTOR automatically chooses the "best" verify algorithm for all nodes 
in a cluster. It does this by parsing /proc/crypto and prioritizing 
accordingly. The problem is that /proc/crypto currently doesn't contain 
information about whether or not an algorithm requires a key – i.e. 
whether or not it is suitable for DRBD's online-verify mechanism.


See this commit for some context:
https://github.com/LINBIT/drbd/commit/34ee32e6922994c8e9390859e1790ca



Also, what about algorithms of type "ahash"?  Shouldn't this field be added for
them too?


You're right. Since we only work with shash in DRBD, I blindly only 
considered this. I will add the field for ahash too.




Also, what about algorithms such as blake2b-256 which optionally take a key (as
indicated by CRYPTO_ALG_OPTIONAL_KEY being set)?  So it's not really "yes" or
"no"; there is a third state as well.


Correct me if I'm missing something, but crypto_shash_alg_needs_key reads:

static inline bool crypto_shash_alg_needs_key(struct shash_alg *alg)
{
return crypto_shash_alg_has_setkey(alg) &&
!(alg->base.cra_flags & CRYPTO_ALG_OPTIONAL_KEY);
}

So this already accounts for optional keys. It just returns "no" for an 
optional key, which seems like reasonable behavior to me (it doesn't 
*need* a key after all).


Another option would be to make it "yes/no/optional". I'm not sure if 
that's more desirable for most people.




- Eric


Thanks,
--
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage

[PATCH] crypto: expose needs_key in procfs

[Christoph Böhmwalder]

Currently, it is not apparent for userspace users which hash algorithms
require a key and which don't. We have /proc/crypto, so add a field
with this information there.

Signed-off-by: Christoph Böhmwalder 

---
 crypto/shash.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/crypto/shash.c b/crypto/shash.c
index 2e3433ad9762..d3127a0618f2 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -477,6 +477,9 @@ static void crypto_shash_show(struct seq_file *m, struct 
crypto_alg *alg)
seq_printf(m, "type : shash\n");
seq_printf(m, "blocksize: %u\n", alg->cra_blocksize);
seq_printf(m, "digestsize   : %u\n", salg->digestsize);
+   seq_printf(m, "needs key: %s\n",
+  crypto_shash_alg_needs_key(salg) ?
+  "yes" : "no");
 }
 
 static const struct crypto_type crypto_shash_type = {
-- 
2.26.2

Re: [PATCH] signal: Allow cifs and drbd to receive their terminating signals

[Christoph Böhmwalder]

On Fri, Aug 16, 2019 at 05:19:38PM -0500, Eric W. Biederman wrote:
> 
> My recent to change to only use force_sig for a synchronous events
> wound up breaking signal reception cifs and drbd.  I had overlooked
> the fact that by default kthreads start out with all signals set to
> SIG_IGN.  So a change I thought was safe turned out to have made it
> impossible for those kernel thread to catch their signals.
> 
> Reverting the work on force_sig is a bad idea because what the code
> was doing was very much a misuse of force_sig.  As the way force_sig
> ultimately allowed the signal to happen was to change the signal
> handler to SIG_DFL.  Which after the first signal will allow userspace
> to send signals to these kernel threads.  At least for
> wake_ack_receiver in drbd that does not appear actively wrong.
> 
> So correct this problem by adding allow_kernel_signal that will allow
> signals whose siginfo reports they were sent by the kernel through,
> but will not allow userspace generated signals, and update cifs and
> drbd to call allow_kernel_signal in an appropriate place so that their
> thread can receive this signal.
> 
> Fixing things this way ensures that userspace won't be able to send
> signals and cause problems, that it is clear which signals the
> threads are expecting to receive, and it guarantees that nothing
> else in the system will be affected.
> 
> This change was partly inspired by similar cifs and drbd patches that
> added allow_signal.
> 
> Reported-by: ronnie sahlberg 
> Reported-by: Christoph Böhmwalder 
> Cc: Steve French 
> Cc: Philipp Reisner 
> Cc: David Laight 
> Fixes: 247bc9470b1e ("cifs: fix rmmod regression in cifs.ko caused by 
> force_sig changes")
> Fixes: 72abe3bcf091 ("signal/cifs: Fix cifs_put_tcp_session to call send_sig 
> instead of force_sig")
> Fixes: fee109901f39 ("signal/drbd: Use send_sig not force_sig")
> Fixes: 3cf5d076fb4d ("signal: Remove task parameter from force_sig")
> Signed-off-by: "Eric W. Biederman" 
> ---
>  drivers/block/drbd/drbd_main.c |  2 ++
>  fs/cifs/connect.c  |  2 +-
>  include/linux/signal.h | 15 ++-
>  kernel/signal.c    |  5 +
>  4 files changed, 22 insertions(+), 2 deletions(-)
> 

Just tested this patch, and I can confirm that it makes DRBD work as
intended again.

Tested-by: Christoph Böhmwalder 

--
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage

Re: [PATCH] drbd: do not ignore signals in threads

[Christoph Böhmwalder]

On 29.07.19 10:50, David Laight wrote:
> Doesn't unmasking the signals and using send_sig() instead  of force_sig()
> have the (probably unwanted) side effect of allowing userspace to send
> the signal?

I have ran some tests, and it does look like it is now possible to send
signals to the DRBD kthread from userspace. However, ...

> I've certainly got some driver code that uses force_sig() on a kthread
> that it doesn't (ever) want userspace to signal.

... we don't feel that it is absolutely necessary for userspace to be
unable to send a signal to our kthreads. This is because the DRBD thread
independently checks its own state, and (for example) only exits as a
result of a signal if its thread state was already "EXITING" to begin
with.

As such, our priority here is to get the main issue -- DRBD hanging upon
exit -- resolved. I agree that it is not exactly desirable to have userspace
send random signals to kthreads; not for DRBD and certainly not in general.
However, we feel like it is more important to have DRBD actually work again
in 5.3.

That said, there should probably still be a way to be able to send a signal
to a kthread from the kernel, but not from userspace. I think the author of
the original patch (Eric) might have some ideas here.

Jens, could you take a look and decide whether or not it's appropriate for you
to funnel this through the linux-block tree to Linus for rc4?

> The origina1 commit says:
>> Further force_sig is for delivering synchronous signals (aka exceptions).
>> The locking in force_sig is not prepared to deal with running processes, as
>> tsk->sighand may change during exec for a running process.
> 
> I think a lot of code has assumed that the only real difference between
> send_sig() and force_sig() is that the latter ignores the signal mask.
> 
> If you need to unblock a kernel thread (eg one blocked in kernel_accept())
> in order to unload a driver, then you really don't want it to be possible
> for anything else to signal the kthread.
> 
>   David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 
> 1PT, UK
> Registration No: 1397386 (Wales)
--
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage

[PATCH] drbd: do not ignore signals in threads

[Christoph Böhmwalder]

Fix a regression introduced by upstream commit fee109901f39
('signal/drbd: Use send_sig not force_sig').

Currently, when a thread is initialized, all signals are set to be
ignored by default. DRBD uses SIGHUP to end its threads, which means it
is now no longer possible to bring down a DRBD resource because the
signals do not make it through to the thread in question.

This circumstance was previously hidden by the fact that DRBD used
force_sig() to kill its threads. The aforementioned upstream commit
changed this to send_sig(), which means the effects of the signals being
ignored by default are now becoming visible.

Thus, issue an allow_signal() at the start of the thread to explicitly
allow the desired signals.

Signed-off-by: Christoph Böhmwalder 
Signed-off-by: Philipp Reisner 
Fixes: fee109901f39 ("signal/drbd: Use send_sig not force_sig")
Cc: sta...@vger.kernel.org
---
 drivers/block/drbd/drbd_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 9bd4ddd12b25..b8b986df6814 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -318,6 +318,9 @@ static int drbd_thread_setup(void *arg)
unsigned long flags;
int retval;
 
+   allow_signal(DRBD_SIGKILL);
+   allow_signal(SIGXCPU);
+
snprintf(current->comm, sizeof(current->comm), "drbd_%c_%s",
 thi->name[0],
 resource->name);
-- 
2.22.0

Re: [PATCH 2/3] dt-bindings: interrupt-controller: RISC-V local interrupt controller docs

[Christoph Böhmwalder]

On Fri, Jun 22, 2018 at 04:20:05PM -0700, Palmer Dabbelt wrote:
> From: Palmer Dabbelt 
> 
> This patch adds documentation on the RISC-V local interrupt controller,
> which is a per-hart interrupt controller that manages all interrupts
> entering a RISC-V hart.  This interrupt controller is present on all
> RISC-V systems.
> 
> Signed-off-by: Palmer Dabbelt 
> ---
>  .../interrupt-controller/riscv,cpu-intc.txt| 41 
> ++
>  1 file changed, 41 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt 
> b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
> new file mode 100644
> index ..61900e2e3868
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
> @@ -0,0 +1,41 @@
> +RISC-V Hart-Level Interrupt Controller (HLIC)
> +-
> +
> +RISC-V cores include Control Status Registers (CSRs) which are local to each
> +hart and can be read or written by software. Some of these CSRs are used to
> +control local interrupts connected to the core.  Every interrupt is 
> ultimately
> +routed through a hart's HLIC before it interrupts that hart.
> +
> +The RISC-V supervisor ISA manual specifies three interrupt sources that are
> +attached to every HLIC: software interrupts, the timer interrupt, and 
> external
> +interrupts.  Software interrupts are used to send IPIs between cores.  The
> +timer interrupt comes from an architecturally mandated real-time timer that 
> is
> +controller via SBI calls and CSR reads.  External interrupts connect all 
> other
> +device interrupts to the HLIC, which are routed via the platform-level
> +interrupt controller (PLIC).
> +
> +All RISC-V systems that conform to the supervisor ISA specification are
> +required to have a HLIC with these three interrupt sources present.  Since 
> the
> +interrupt map is defined by the ISA it's not listed in the HLIC's device tree
> +entry, though external interrupt controllers (like the PLIC, for example) 
> will
> +need to define how their interrupts map to the relevant HLICs.
> +
> +Required properties:
> +- compatible : "riscv,cpu-intc"
> +- #interrupt-cells : should be <1>
> +- interrupt-controller : Identifies the node as an interrupt controller
> +
> +Furthermore, this interrupt-controller MUST be embedded inside the cpu
> +definition of the hart whose CSRs control these local interrupts.
> +
> +An example device tree entry for a HLIC is show below.

Spotted a typo here, "show" -> "shown".

> +
> + cpu1: cpu@1 {
> + compatible = "riscv";
> + ...
> + cpu1-intc: interrupt-controller {
> + #interrupt-cells = <1>;
> + compatible = "riscv,cpu-intc";
> + interrupt-controller;
> + };
> + };
> -- 
> 2.16.4

Also, I've noticed that double spaces after punctuation are used pretty
inconsistently throughout the document. Is that intended?

--
Regards,
Christoph

Re: [PATCH 2/3] dt-bindings: interrupt-controller: RISC-V local interrupt controller docs

[Christoph Böhmwalder]

On Fri, Jun 22, 2018 at 04:20:05PM -0700, Palmer Dabbelt wrote:
> From: Palmer Dabbelt 
> 
> This patch adds documentation on the RISC-V local interrupt controller,
> which is a per-hart interrupt controller that manages all interrupts
> entering a RISC-V hart.  This interrupt controller is present on all
> RISC-V systems.
> 
> Signed-off-by: Palmer Dabbelt 
> ---
>  .../interrupt-controller/riscv,cpu-intc.txt| 41 
> ++
>  1 file changed, 41 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt 
> b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
> new file mode 100644
> index ..61900e2e3868
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
> @@ -0,0 +1,41 @@
> +RISC-V Hart-Level Interrupt Controller (HLIC)
> +-
> +
> +RISC-V cores include Control Status Registers (CSRs) which are local to each
> +hart and can be read or written by software. Some of these CSRs are used to
> +control local interrupts connected to the core.  Every interrupt is 
> ultimately
> +routed through a hart's HLIC before it interrupts that hart.
> +
> +The RISC-V supervisor ISA manual specifies three interrupt sources that are
> +attached to every HLIC: software interrupts, the timer interrupt, and 
> external
> +interrupts.  Software interrupts are used to send IPIs between cores.  The
> +timer interrupt comes from an architecturally mandated real-time timer that 
> is
> +controller via SBI calls and CSR reads.  External interrupts connect all 
> other
> +device interrupts to the HLIC, which are routed via the platform-level
> +interrupt controller (PLIC).
> +
> +All RISC-V systems that conform to the supervisor ISA specification are
> +required to have a HLIC with these three interrupt sources present.  Since 
> the
> +interrupt map is defined by the ISA it's not listed in the HLIC's device tree
> +entry, though external interrupt controllers (like the PLIC, for example) 
> will
> +need to define how their interrupts map to the relevant HLICs.
> +
> +Required properties:
> +- compatible : "riscv,cpu-intc"
> +- #interrupt-cells : should be <1>
> +- interrupt-controller : Identifies the node as an interrupt controller
> +
> +Furthermore, this interrupt-controller MUST be embedded inside the cpu
> +definition of the hart whose CSRs control these local interrupts.
> +
> +An example device tree entry for a HLIC is show below.

Spotted a typo here, "show" -> "shown".

> +
> + cpu1: cpu@1 {
> + compatible = "riscv";
> + ...
> + cpu1-intc: interrupt-controller {
> + #interrupt-cells = <1>;
> + compatible = "riscv,cpu-intc";
> + interrupt-controller;
> + };
> + };
> -- 
> 2.16.4

Also, I've noticed that double spaces after punctuation are used pretty
inconsistently throughout the document. Is that intended?

--
Regards,
Christoph

Re: [PATCH 3/3] hid: logitech-dj: delete unnecessary error messages

[Christoph Böhmwalder]

On Thu, Feb 08, 2018 at 08:56:28AM +0100, Marcus Folkesson wrote:
> 
> Thank you, but Markus Elfring already has a submitted a patch for this one.
> 
> /Marcus

Ah sorry, I must've missed that one.  Feel free to dismiss it then (it's
obviously independent of the other two anyways).

--
Regards,
Christoph

Re: [PATCH 3/3] hid: logitech-dj: delete unnecessary error messages

[Christoph Böhmwalder]

On Thu, Feb 08, 2018 at 08:56:28AM +0100, Marcus Folkesson wrote:
> 
> Thank you, but Markus Elfring already has a submitted a patch for this one.
> 
> /Marcus

Ah sorry, I must've missed that one.  Feel free to dismiss it then (it's
obviously independent of the other two anyways).

--
Regards,
Christoph

[PATCH 2/3] hid: logitech-dj: fix checkpatch issues

[Christoph Böhmwalder]

Fix some code style issues, mostly related to making sure quoted strings
aren't split over multiple lines.

Other fixes:
* Drop != NULL from some null pointer checks
* Use sizeof(*ptr) instead of sizeof(ptr_type)
* Combine some if statements to reduce indentation

Signed-off-by: Christoph Böhmwalder <christ...@boehmwalder.at>
---
 drivers/hid/hid-logitech-dj.c | 71 ++-
 1 file changed, 36 insertions(+), 35 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 530d10b5a404..59c54cb4bc64 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -347,13 +347,14 @@ static void logi_dj_recv_destroy_djhid_device(struct 
dj_receiver_dev *djrcv_dev,
djrcv_dev->paired_dj_devices[dj_report->device_index] = NULL;
spin_unlock_irqrestore(_dev->lock, flags);
 
-   if (dj_dev != NULL) {
+   if (dj_dev) {
hid_destroy_device(dj_dev->hdev);
kfree(dj_dev);
-   } else {
-   dev_err(_dev->hdev->dev, "%s: can't destroy a NULL 
device\n",
-   __func__);
+   return;
}
+
+   dev_err(_dev->hdev->dev, "%s: can't destroy a NULL device\n",
+   __func__);
 }
 
 static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
@@ -411,7 +412,7 @@ static void logi_dj_recv_add_djhid_device(struct 
dj_receiver_dev *djrcv_dev,
snprintf(tmpstr, sizeof(tmpstr), ":%d", dj_report->device_index);
strlcat(dj_hiddev->phys, tmpstr, sizeof(dj_hiddev->phys));
 
-   dj_dev = kzalloc(sizeof(struct dj_device), GFP_KERNEL);
+   dj_dev = kzalloc(sizeof(*dj_dev), GFP_KERNEL);
 
if (!dj_dev) {
dev_err(_hdev->dev, "%s: failed allocating dj_device\n",
@@ -419,8 +420,9 @@ static void logi_dj_recv_add_djhid_device(struct 
dj_receiver_dev *djrcv_dev,
goto dj_device_allocate_fail;
}
 
-   dj_dev->reports_supported = get_unaligned_le32(
-   dj_report->report_params + DEVICE_PAIRED_RF_REPORT_TYPE);
+   dj_dev->reports_supported =
+   get_unaligned_le32(dj_report->report_params
+  + DEVICE_PAIRED_RF_REPORT_TYPE);
dj_dev->hdev = dj_hiddev;
dj_dev->dj_receiver_dev = djrcv_dev;
dj_dev->device_index = dj_report->device_index;
@@ -461,17 +463,17 @@ static void delayedwork_callback(struct work_struct *work)
  sizeof(struct dj_report));
 
if (count != sizeof(struct dj_report)) {
-   dev_err(_dev->hdev->dev, "%s: workitem triggered without "
-   "notifications available\n", __func__);
+   dev_err(_dev->hdev->dev,
+   "%s: workitem triggered without notifications available"
+   "\n", __func__);
spin_unlock_irqrestore(_dev->lock, flags);
return;
}
 
-   if (!kfifo_is_empty(_dev->notif_fifo)) {
-   if (schedule_work(_dev->work) == 0) {
-   dbg_hid("%s: did not schedule the work item, was "
-   "already queued\n", __func__);
-   }
+   if (!kfifo_is_empty(_dev->notif_fifo) &&
+   schedule_work(_dev->work) == 0) {
+   dbg_hid("%s: did not schedule the work item, was already queued"
+   "\n", __func__);
}
 
spin_unlock_irqrestore(_dev->lock, flags);
@@ -504,8 +506,8 @@ static void delayedwork_callback(struct work_struct *work)
break;
}
dev_err(_dev->hdev->dev,
-   "%s:logi_dj_recv_query_paired_devices"
-   " error:%d\n", __func__, retval);
+   "%s: logi_dj_recv_query_paired_devices error: "
+   "%d\n", __func__, retval);
}
dbg_hid("%s: unexpected report type\n", __func__);
}
@@ -519,8 +521,8 @@ static void logi_dj_recv_queue_notification(struct 
dj_receiver_dev *djrcv_dev,
kfifo_in(_dev->notif_fifo, dj_report, sizeof(struct dj_report));
 
if (schedule_work(_dev->work) == 0) {
-   dbg_hid("%s: did not schedule the work item, was already "
-   "queued\n", __func__);
+   dbg_hid("%s: did not schedule the work item, was already queued"
+   "\n", __func__);
}
 }
 
@@ -543,8 +545,7 @@ static void logi_dj_recv_forward_null_report(struct 
dj_receiver_dev *djrcv_dev,
 

[PATCH 2/3] hid: logitech-dj: fix checkpatch issues

[Christoph Böhmwalder]

Fix some code style issues, mostly related to making sure quoted strings
aren't split over multiple lines.

Other fixes:
* Drop != NULL from some null pointer checks
* Use sizeof(*ptr) instead of sizeof(ptr_type)
* Combine some if statements to reduce indentation

Signed-off-by: Christoph Böhmwalder 
---
 drivers/hid/hid-logitech-dj.c | 71 ++-
 1 file changed, 36 insertions(+), 35 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 530d10b5a404..59c54cb4bc64 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -347,13 +347,14 @@ static void logi_dj_recv_destroy_djhid_device(struct 
dj_receiver_dev *djrcv_dev,
djrcv_dev->paired_dj_devices[dj_report->device_index] = NULL;
spin_unlock_irqrestore(_dev->lock, flags);
 
-   if (dj_dev != NULL) {
+   if (dj_dev) {
hid_destroy_device(dj_dev->hdev);
kfree(dj_dev);
-   } else {
-   dev_err(_dev->hdev->dev, "%s: can't destroy a NULL 
device\n",
-   __func__);
+   return;
}
+
+   dev_err(_dev->hdev->dev, "%s: can't destroy a NULL device\n",
+   __func__);
 }
 
 static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
@@ -411,7 +412,7 @@ static void logi_dj_recv_add_djhid_device(struct 
dj_receiver_dev *djrcv_dev,
snprintf(tmpstr, sizeof(tmpstr), ":%d", dj_report->device_index);
strlcat(dj_hiddev->phys, tmpstr, sizeof(dj_hiddev->phys));
 
-   dj_dev = kzalloc(sizeof(struct dj_device), GFP_KERNEL);
+   dj_dev = kzalloc(sizeof(*dj_dev), GFP_KERNEL);
 
if (!dj_dev) {
dev_err(_hdev->dev, "%s: failed allocating dj_device\n",
@@ -419,8 +420,9 @@ static void logi_dj_recv_add_djhid_device(struct 
dj_receiver_dev *djrcv_dev,
goto dj_device_allocate_fail;
}
 
-   dj_dev->reports_supported = get_unaligned_le32(
-   dj_report->report_params + DEVICE_PAIRED_RF_REPORT_TYPE);
+   dj_dev->reports_supported =
+   get_unaligned_le32(dj_report->report_params
+  + DEVICE_PAIRED_RF_REPORT_TYPE);
dj_dev->hdev = dj_hiddev;
dj_dev->dj_receiver_dev = djrcv_dev;
dj_dev->device_index = dj_report->device_index;
@@ -461,17 +463,17 @@ static void delayedwork_callback(struct work_struct *work)
  sizeof(struct dj_report));
 
if (count != sizeof(struct dj_report)) {
-   dev_err(_dev->hdev->dev, "%s: workitem triggered without "
-   "notifications available\n", __func__);
+   dev_err(_dev->hdev->dev,
+   "%s: workitem triggered without notifications available"
+   "\n", __func__);
spin_unlock_irqrestore(_dev->lock, flags);
return;
}
 
-   if (!kfifo_is_empty(_dev->notif_fifo)) {
-   if (schedule_work(_dev->work) == 0) {
-   dbg_hid("%s: did not schedule the work item, was "
-   "already queued\n", __func__);
-   }
+   if (!kfifo_is_empty(_dev->notif_fifo) &&
+   schedule_work(_dev->work) == 0) {
+   dbg_hid("%s: did not schedule the work item, was already queued"
+   "\n", __func__);
}
 
spin_unlock_irqrestore(_dev->lock, flags);
@@ -504,8 +506,8 @@ static void delayedwork_callback(struct work_struct *work)
break;
}
dev_err(_dev->hdev->dev,
-   "%s:logi_dj_recv_query_paired_devices"
-   " error:%d\n", __func__, retval);
+   "%s: logi_dj_recv_query_paired_devices error: "
+   "%d\n", __func__, retval);
}
dbg_hid("%s: unexpected report type\n", __func__);
}
@@ -519,8 +521,8 @@ static void logi_dj_recv_queue_notification(struct 
dj_receiver_dev *djrcv_dev,
kfifo_in(_dev->notif_fifo, dj_report, sizeof(struct dj_report));
 
if (schedule_work(_dev->work) == 0) {
-   dbg_hid("%s: did not schedule the work item, was already "
-   "queued\n", __func__);
+   dbg_hid("%s: did not schedule the work item, was already queued"
+   "\n", __func__);
}
 }
 
@@ -543,8 +545,7 @@ static void logi_dj_recv_forward_null_report(struct 
dj_receiver_dev *djrcv_dev,
   

[PATCH 1/3] hid: logitech-dj: fix various style issues

[Christoph Böhmwalder]

Fix some problems regarding comment/whitespace style.  Mostly reported
by checkpatch.pl

Individual changes:
* Remove paragraph about writing to the FSF in GPL header
* Fix several spelling and grammar mistakes in comments
* Fix various misalignments
* Remove some unnecessary blank lines
* Adapt comment style to fit the kernel coding standard

Signed-off-by: Christoph Böhmwalder <christ...@boehmwalder.at>
---
 drivers/hid/hid-logitech-dj.c | 240 ++
 1 file changed, 124 insertions(+), 116 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 826fa1e1c8d9..530d10b5a404 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -14,14 +14,8 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
  */
 
-
 #include 
 #include 
 #include 
@@ -53,13 +47,13 @@
 #define REPORT_TYPE_RFREPORT_FIRST 0x01
 #define REPORT_TYPE_RFREPORT_LAST  0x1F
 
-/* Command Switch to DJ mode */
+/* Command to switch to DJ mode */
 #define REPORT_TYPE_CMD_SWITCH 0x80
 #define CMD_SWITCH_PARAM_DEVBITFIELD   0x00
 #define CMD_SWITCH_PARAM_TIMEOUT_SECONDS   0x01
 #define TIMEOUT_NO_KEEPALIVE   0x00
 
-/* Command to Get the list of Paired devices */
+/* Command to get the list of paired devices */
 #define REPORT_TYPE_CMD_GET_PAIRED_DEVICES 0x81
 
 /* Device Paired Notification */
@@ -74,7 +68,6 @@
 /* Device Un-Paired Notification */
 #define REPORT_TYPE_NOTIF_DEVICE_UNPAIRED  0x40
 
-
 /* Connection Status Notification */
 #define REPORT_TYPE_NOTIF_CONNECTION_STATUS0x42
 #define CONNECTION_STATUS_PARAM_STATUS 0x00
@@ -85,7 +78,7 @@
 #define NOTIF_ERROR_PARAM_ETYPE0x00
 #define ETYPE_KEEPALIVE_TIMEOUT0x01
 
-/* supported DJ HID && RF report types */
+/* Supported DJ HID & RF report types */
 #define REPORT_TYPE_KEYBOARD   0x01
 #define REPORT_TYPE_MOUSE  0x02
 #define REPORT_TYPE_CONSUMER_CONTROL   0x03
@@ -127,38 +120,38 @@ struct dj_device {
 
 /* Keyboard descriptor (1) */
 static const char kbd_descriptor[] = {
-   0x05, 0x01, /* USAGE_PAGE (generic Desktop) */
-   0x09, 0x06, /* USAGE (Keyboard) */
-   0xA1, 0x01, /* COLLECTION (Application) */
-   0x85, 0x01, /* REPORT_ID (1)*/
-   0x95, 0x08, /*   REPORT_COUNT (8)   */
-   0x75, 0x01, /*   REPORT_SIZE (1)*/
-   0x15, 0x00, /*   LOGICAL_MINIMUM (0)*/
-   0x25, 0x01, /*   LOGICAL_MAXIMUM (1)*/
-   0x05, 0x07, /*   USAGE_PAGE (Keyboard)  */
-   0x19, 0xE0, /*   USAGE_MINIMUM (Left Control)   */
-   0x29, 0xE7, /*   USAGE_MAXIMUM (Right GUI)  */
-   0x81, 0x02, /*   INPUT (Data,Var,Abs)   */
-   0x95, 0x06, /*   REPORT_COUNT (6)   */
-   0x75, 0x08, /*   REPORT_SIZE (8)*/
-   0x15, 0x00, /*   LOGICAL_MINIMUM (0)*/
-   0x26, 0xFF, 0x00,   /*   LOGICAL_MAXIMUM (255)  */
-   0x05, 0x07, /*   USAGE_PAGE (Keyboard)  */
-   0x19, 0x00, /*   USAGE_MINIMUM (no event)   */
-   0x2A, 0xFF, 0x00,   /*   USAGE_MAXIMUM (reserved)   */
-   0x81, 0x00, /*   INPUT (Data,Ary,Abs)   */
-   0x85, 0x0e, /* REPORT_ID (14)   */
-   0x05, 0x08, /*   USAGE PAGE (LED page)  */
-   0x95, 0x05, /*   REPORT COUNT (5)   */
-   0x75, 0x01, /*   REPORT SIZE (1)*/
-   0x15, 0x00, /*   LOGICAL_MINIMUM (0)*/
-   0x25, 0x01, /*   LOGICAL_MAXIMUM (1)*/
-   0x19, 0x01, /*   USAGE MINIMUM (1)  */
-   0x29, 0x05, /*   USAGE MAXIMUM (5)  */
+   0x05, 0x01, /* USAGE_PAGE (generic Desktop) */
+   0x09, 0x06, /* USAGE (Keyboard) */
+   0xA1, 0x01, /* COLLECTION (Application) */
+   0x85, 0x01, /* REPORT_ID (1)*/
+   0x95, 0x08, /*   REPORT_COUNT (8)   */
+   0x75, 0x01, /*   REPORT_SIZE (1)*/
+   0x15, 0x00, /*   LOGICAL_MINIMUM (0)*/

[PATCH 1/3] hid: logitech-dj: fix various style issues

[Christoph Böhmwalder]

Fix some problems regarding comment/whitespace style.  Mostly reported
by checkpatch.pl

Individual changes:
* Remove paragraph about writing to the FSF in GPL header
* Fix several spelling and grammar mistakes in comments
* Fix various misalignments
* Remove some unnecessary blank lines
* Adapt comment style to fit the kernel coding standard

Signed-off-by: Christoph Böhmwalder 
---
 drivers/hid/hid-logitech-dj.c | 240 ++
 1 file changed, 124 insertions(+), 116 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 826fa1e1c8d9..530d10b5a404 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -14,14 +14,8 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
  */
 
-
 #include 
 #include 
 #include 
@@ -53,13 +47,13 @@
 #define REPORT_TYPE_RFREPORT_FIRST 0x01
 #define REPORT_TYPE_RFREPORT_LAST  0x1F
 
-/* Command Switch to DJ mode */
+/* Command to switch to DJ mode */
 #define REPORT_TYPE_CMD_SWITCH 0x80
 #define CMD_SWITCH_PARAM_DEVBITFIELD   0x00
 #define CMD_SWITCH_PARAM_TIMEOUT_SECONDS   0x01
 #define TIMEOUT_NO_KEEPALIVE   0x00
 
-/* Command to Get the list of Paired devices */
+/* Command to get the list of paired devices */
 #define REPORT_TYPE_CMD_GET_PAIRED_DEVICES 0x81
 
 /* Device Paired Notification */
@@ -74,7 +68,6 @@
 /* Device Un-Paired Notification */
 #define REPORT_TYPE_NOTIF_DEVICE_UNPAIRED  0x40
 
-
 /* Connection Status Notification */
 #define REPORT_TYPE_NOTIF_CONNECTION_STATUS0x42
 #define CONNECTION_STATUS_PARAM_STATUS 0x00
@@ -85,7 +78,7 @@
 #define NOTIF_ERROR_PARAM_ETYPE0x00
 #define ETYPE_KEEPALIVE_TIMEOUT0x01
 
-/* supported DJ HID && RF report types */
+/* Supported DJ HID & RF report types */
 #define REPORT_TYPE_KEYBOARD   0x01
 #define REPORT_TYPE_MOUSE  0x02
 #define REPORT_TYPE_CONSUMER_CONTROL   0x03
@@ -127,38 +120,38 @@ struct dj_device {
 
 /* Keyboard descriptor (1) */
 static const char kbd_descriptor[] = {
-   0x05, 0x01, /* USAGE_PAGE (generic Desktop) */
-   0x09, 0x06, /* USAGE (Keyboard) */
-   0xA1, 0x01, /* COLLECTION (Application) */
-   0x85, 0x01, /* REPORT_ID (1)*/
-   0x95, 0x08, /*   REPORT_COUNT (8)   */
-   0x75, 0x01, /*   REPORT_SIZE (1)*/
-   0x15, 0x00, /*   LOGICAL_MINIMUM (0)*/
-   0x25, 0x01, /*   LOGICAL_MAXIMUM (1)*/
-   0x05, 0x07, /*   USAGE_PAGE (Keyboard)  */
-   0x19, 0xE0, /*   USAGE_MINIMUM (Left Control)   */
-   0x29, 0xE7, /*   USAGE_MAXIMUM (Right GUI)  */
-   0x81, 0x02, /*   INPUT (Data,Var,Abs)   */
-   0x95, 0x06, /*   REPORT_COUNT (6)   */
-   0x75, 0x08, /*   REPORT_SIZE (8)*/
-   0x15, 0x00, /*   LOGICAL_MINIMUM (0)*/
-   0x26, 0xFF, 0x00,   /*   LOGICAL_MAXIMUM (255)  */
-   0x05, 0x07, /*   USAGE_PAGE (Keyboard)  */
-   0x19, 0x00, /*   USAGE_MINIMUM (no event)   */
-   0x2A, 0xFF, 0x00,   /*   USAGE_MAXIMUM (reserved)   */
-   0x81, 0x00, /*   INPUT (Data,Ary,Abs)   */
-   0x85, 0x0e, /* REPORT_ID (14)   */
-   0x05, 0x08, /*   USAGE PAGE (LED page)  */
-   0x95, 0x05, /*   REPORT COUNT (5)   */
-   0x75, 0x01, /*   REPORT SIZE (1)*/
-   0x15, 0x00, /*   LOGICAL_MINIMUM (0)*/
-   0x25, 0x01, /*   LOGICAL_MAXIMUM (1)*/
-   0x19, 0x01, /*   USAGE MINIMUM (1)  */
-   0x29, 0x05, /*   USAGE MAXIMUM (5)  */
+   0x05, 0x01, /* USAGE_PAGE (generic Desktop) */
+   0x09, 0x06, /* USAGE (Keyboard) */
+   0xA1, 0x01, /* COLLECTION (Application) */
+   0x85, 0x01, /* REPORT_ID (1)*/
+   0x95, 0x08, /*   REPORT_COUNT (8)   */
+   0x75, 0x01, /*   REPORT_SIZE (1)*/
+   0x15, 0x00, /*   LOGICAL_MINIMUM (0)*/
+   0x25, 0x01, /*   LOGICA

[PATCH 0/3] hid: logitech-dj: code style improvements

[Christoph Böhmwalder]

Fix several rather trivial code style issues in the Logitech DJ HID
driver.  Most of these were reported by checkpatch, others are just
attempts to make the code adhere more to the kernel coding guidelines.

Signed-off-by: Christoph Böhmwalder <christ...@boehmwalder.at>

Christoph Böhmwalder (3):
  hid: logitech-dj: fix various style issues
  hid: logitech-dj: fix checkpatch issues
  hid: logitech-dj: delete unnecessary error messages

 drivers/hid/hid-logitech-dj.c | 318 +-
 1 file changed, 161 insertions(+), 157 deletions(-)

-- 
2.13.6

[PATCH 0/3] hid: logitech-dj: code style improvements

[Christoph Böhmwalder]

Fix several rather trivial code style issues in the Logitech DJ HID
driver.  Most of these were reported by checkpatch, others are just
attempts to make the code adhere more to the kernel coding guidelines.

Signed-off-by: Christoph Böhmwalder 

Christoph Böhmwalder (3):
  hid: logitech-dj: fix various style issues
  hid: logitech-dj: fix checkpatch issues
  hid: logitech-dj: delete unnecessary error messages

 drivers/hid/hid-logitech-dj.c | 318 +-
 1 file changed, 161 insertions(+), 157 deletions(-)

-- 
2.13.6

[PATCH 3/3] hid: logitech-dj: delete unnecessary error messages

[Christoph Böhmwalder]

Remove some "out of memory" messages that are considered useless.

Signed-off-by: Christoph Böhmwalder <christ...@boehmwalder.at>
---
 drivers/hid/hid-logitech-dj.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 59c54cb4bc64..ba5239840cda 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -414,11 +414,8 @@ static void logi_dj_recv_add_djhid_device(struct 
dj_receiver_dev *djrcv_dev,
 
dj_dev = kzalloc(sizeof(*dj_dev), GFP_KERNEL);
 
-   if (!dj_dev) {
-   dev_err(_hdev->dev, "%s: failed allocating dj_device\n",
-   __func__);
+   if (!dj_dev)
goto dj_device_allocate_fail;
-   }
 
dj_dev->reports_supported =
get_unaligned_le32(dj_report->report_params
@@ -1015,11 +1012,9 @@ static int logi_dj_probe(struct hid_device *hdev,
/* Treat interface 2 */
 
djrcv_dev = kzalloc(sizeof(*djrcv_dev), GFP_KERNEL);
-   if (!djrcv_dev) {
-   dev_err(>dev,
-   "%s:failed allocating dj_receiver_dev\n", __func__);
+   if (!djrcv_dev)
return -ENOMEM;
-   }
+
djrcv_dev->hdev = hdev;
INIT_WORK(_dev->work, delayedwork_callback);
spin_lock_init(_dev->lock);
-- 
2.13.6

[PATCH 3/3] hid: logitech-dj: delete unnecessary error messages

[Christoph Böhmwalder]

Remove some "out of memory" messages that are considered useless.

Signed-off-by: Christoph Böhmwalder 
---
 drivers/hid/hid-logitech-dj.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 59c54cb4bc64..ba5239840cda 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -414,11 +414,8 @@ static void logi_dj_recv_add_djhid_device(struct 
dj_receiver_dev *djrcv_dev,
 
dj_dev = kzalloc(sizeof(*dj_dev), GFP_KERNEL);
 
-   if (!dj_dev) {
-   dev_err(_hdev->dev, "%s: failed allocating dj_device\n",
-   __func__);
+   if (!dj_dev)
goto dj_device_allocate_fail;
-   }
 
dj_dev->reports_supported =
get_unaligned_le32(dj_report->report_params
@@ -1015,11 +1012,9 @@ static int logi_dj_probe(struct hid_device *hdev,
/* Treat interface 2 */
 
djrcv_dev = kzalloc(sizeof(*djrcv_dev), GFP_KERNEL);
-   if (!djrcv_dev) {
-   dev_err(>dev,
-   "%s:failed allocating dj_receiver_dev\n", __func__);
+   if (!djrcv_dev)
return -ENOMEM;
-   }
+
djrcv_dev->hdev = hdev;
INIT_WORK(_dev->work, delayedwork_callback);
spin_lock_init(_dev->lock);
-- 
2.13.6

Re: [PATCH v3 1/2] eeprom: at24: fix coding style issues

[Christoph Böhmwalder]

On Thu, Dec 07, 2017 at 02:39:14PM +0100, Bartosz Golaszewski wrote:
> -#define AT24_DEVICE_MAGIC(_len, _flags)  \
> - ((1 << AT24_SIZE_FLAGS | (_flags))  \
> +#define AT24_DEVICE_MAGIC(_len, _flags)  \
> + ((1 << AT24_SIZE_FLAGS | (_flags))  \
>   << AT24_SIZE_BYTELEN | ilog2(_len))

Looks like there's been a whitespace accident on that first added line.
(Backslash has one more tab in front of it than it should have)

--
Regards,
Christoph

Re: [PATCH v3 1/2] eeprom: at24: fix coding style issues

[Christoph Böhmwalder]

On Thu, Dec 07, 2017 at 02:39:14PM +0100, Bartosz Golaszewski wrote:
> -#define AT24_DEVICE_MAGIC(_len, _flags)  \
> - ((1 << AT24_SIZE_FLAGS | (_flags))  \
> +#define AT24_DEVICE_MAGIC(_len, _flags)  \
> + ((1 << AT24_SIZE_FLAGS | (_flags))  \
>   << AT24_SIZE_BYTELEN | ilog2(_len))

Looks like there's been a whitespace accident on that first added line.
(Backslash has one more tab in front of it than it should have)

--
Regards,
Christoph

Re: [PATCH 2/3] battery: Add the ThinkPad "Not Charging" quirk

[Christoph Böhmwalder]

On Wed, Dec 06, 2017 at 11:53:01PM +0100, Ognjen Galic wrote:
> The EC/ACPI firmware on Lenovo ThinkPads used to report a status
> of "Unknown" when the battery is between the charge start and
> charge stop thresholds. On Windows, it reports "Not Charging"
> so the quirk has been added to also report correctly.
>
> Now the "status" attribute returns "Not Charging" when the
> battery on ThinkPads is not physicaly charging.

Hi again,

I just tried this patch(set) and it still seems to report "Unknown".  We
can also discuss this on the IRC you mentioned earlier if you want, as I
said I'm open for testing.

--
Regards,
Christoph

Re: [PATCH 2/3] battery: Add the ThinkPad "Not Charging" quirk

[Christoph Böhmwalder]

On Wed, Dec 06, 2017 at 11:53:01PM +0100, Ognjen Galic wrote:
> The EC/ACPI firmware on Lenovo ThinkPads used to report a status
> of "Unknown" when the battery is between the charge start and
> charge stop thresholds. On Windows, it reports "Not Charging"
> so the quirk has been added to also report correctly.
>
> Now the "status" attribute returns "Not Charging" when the
> battery on ThinkPads is not physicaly charging.

Hi again,

I just tried this patch(set) and it still seems to report "Unknown".  We
can also discuss this on the IRC you mentioned earlier if you want, as I
said I'm open for testing.

--
Regards,
Christoph

Re: [PATCH v2] thinkad_acpi: Support the battery wear control

[Christoph Böhmwalder]

On Sun, Dec 03, 2017 at 11:56:40PM +0100, Ognjen Galic wrote:
> Add support for the ACPI batteries on newer thinkpad models
> (>Sandy Bridge) that support the setting of start and stop
> thresholds.
> 
> The actual interface to the driver is a extension for the
> existing ACPI battery driver. This was done so that users
> can write transparently to the interface of the ACPI battery
> driver and dont have to use some private interface
> (for ex. /sys/devices/platform/thinkpad_acpi/).
> 
> Two new interfaces are created:
> 
> /sys/class/power_supply/BAT{0,1}/charge_start_threshold
> /sys/class/power_supply/BAT{0,1}/charge_stop_threshold

Just tried this on my X1 Carbon (i7-3667U Ivy Lake):

# cat /sys/class/power_supply/BAT0/charge_stop_threshold
100
# cat /sys/class/power_supply/BAT0/charge_start_threshold
100

That doesn't seem to make any sense.  Is my battery somehow reporting
false values here?  Any way to cross check these values?

--
Regards,
Christoph

Re: [PATCH v2] thinkad_acpi: Support the battery wear control

[Christoph Böhmwalder]

On Sun, Dec 03, 2017 at 11:56:40PM +0100, Ognjen Galic wrote:
> Add support for the ACPI batteries on newer thinkpad models
> (>Sandy Bridge) that support the setting of start and stop
> thresholds.
> 
> The actual interface to the driver is a extension for the
> existing ACPI battery driver. This was done so that users
> can write transparently to the interface of the ACPI battery
> driver and dont have to use some private interface
> (for ex. /sys/devices/platform/thinkpad_acpi/).
> 
> Two new interfaces are created:
> 
> /sys/class/power_supply/BAT{0,1}/charge_start_threshold
> /sys/class/power_supply/BAT{0,1}/charge_stop_threshold

Just tried this on my X1 Carbon (i7-3667U Ivy Lake):

# cat /sys/class/power_supply/BAT0/charge_stop_threshold
100
# cat /sys/class/power_supply/BAT0/charge_start_threshold
100

That doesn't seem to make any sense.  Is my battery somehow reporting
false values here?  Any way to cross check these values?

--
Regards,
Christoph

[PATCH] drm/nouveau/mmu: fix odd_ptr_err.cocci warnings

[Christoph Böhmwalder]

The kbuild test bot complained about a new coccinelle warning nearby,
which sparked a discussion about the assignment to 'memory' inside of
the conditional expression.  See Link below for the original post.

Fix the assignment to silence the coccinelle warning and also make the
code look a little nicer.

Link: https://lists.freedesktop.org/archives/nouveau/2017-November/029242.html
Signed-off-by: Christoph Böhmwalder <christ...@boehmwalder.at>
---
 drm/nouveau/nvkm/subdev/mmu/uvmm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drm/nouveau/nvkm/subdev/mmu/uvmm.c 
b/drm/nouveau/nvkm/subdev/mmu/uvmm.c
index fa81d0c1..37b201b9 100644
--- a/drm/nouveau/nvkm/subdev/mmu/uvmm.c
+++ b/drm/nouveau/nvkm/subdev/mmu/uvmm.c
@@ -106,7 +106,8 @@ nvkm_uvmm_mthd_map(struct nvkm_uvmm *uvmm, void *argv, u32 
argc)
} else
return ret;
 
-   if (IS_ERR((memory = nvkm_umem_search(client, handle {
+   memory = nvkm_umem_search(client, handle);
+   if (IS_ERR(memory)) {
VMM_DEBUG(vmm, "memory %016llx %ld\n", handle, PTR_ERR(memory));
return PTR_ERR(memory);
}
-- 
2.13.6

[PATCH] drm/nouveau/mmu: fix odd_ptr_err.cocci warnings

[Christoph Böhmwalder]

The kbuild test bot complained about a new coccinelle warning nearby,
which sparked a discussion about the assignment to 'memory' inside of
the conditional expression.  See Link below for the original post.

Fix the assignment to silence the coccinelle warning and also make the
code look a little nicer.

Link: https://lists.freedesktop.org/archives/nouveau/2017-November/029242.html
Signed-off-by: Christoph Böhmwalder 
---
 drm/nouveau/nvkm/subdev/mmu/uvmm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drm/nouveau/nvkm/subdev/mmu/uvmm.c 
b/drm/nouveau/nvkm/subdev/mmu/uvmm.c
index fa81d0c1..37b201b9 100644
--- a/drm/nouveau/nvkm/subdev/mmu/uvmm.c
+++ b/drm/nouveau/nvkm/subdev/mmu/uvmm.c
@@ -106,7 +106,8 @@ nvkm_uvmm_mthd_map(struct nvkm_uvmm *uvmm, void *argv, u32 
argc)
} else
return ret;
 
-   if (IS_ERR((memory = nvkm_umem_search(client, handle {
+   memory = nvkm_umem_search(client, handle);
+   if (IS_ERR(memory)) {
VMM_DEBUG(vmm, "memory %016llx %ld\n", handle, PTR_ERR(memory));
return PTR_ERR(memory);
}
-- 
2.13.6

[PATCH 2/3] wireless: iwlwifi: function definition cosmetic fix

[Christoph Böhmwalder]

Separate the function from the previous definition with a newline and
put the `static` keyword on the same line, as it just looks nicer.

Signed-off-by: Christoph Böhmwalder <christ...@boehmwalder.at>
---
 drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
index 0eb815ae97e8..249ee1c7b02f 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
@@ -325,8 +325,8 @@ static u16 channel_id_to_txp(struct iwl_phy_db *phy_db, u16 
ch_id)
}
return 0xff;
 }
-static
-int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
+
+static int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
u32 type, u8 **data, u16 *size, u16 ch_id)
 {
struct iwl_phy_db_entry *entry;
-- 
2.13.5

[PATCH 2/3] wireless: iwlwifi: function definition cosmetic fix

[Christoph Böhmwalder]

Separate the function from the previous definition with a newline and
put the `static` keyword on the same line, as it just looks nicer.

Signed-off-by: Christoph Böhmwalder 
---
 drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
index 0eb815ae97e8..249ee1c7b02f 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
@@ -325,8 +325,8 @@ static u16 channel_id_to_txp(struct iwl_phy_db *phy_db, u16 
ch_id)
}
return 0xff;
 }
-static
-int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
+
+static int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
u32 type, u8 **data, u16 *size, u16 ch_id)
 {
struct iwl_phy_db_entry *entry;
-- 
2.13.5

[PATCH 1/3] wireless: iwlwifi: use bool instead of int

[Christoph Böhmwalder]

Change a usage of int in a boolean context to use the bool type instead, as it
makes the intent of the function clearer and helps clarify its semantics.

Also eliminate the if/else and just return the boolean result directly,
making the code more readable.

Signed-off-by: Christoph Böhmwalder <christ...@boehmwalder.at>
---
 drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
index b7cd813ba70f..0eb815ae97e8 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
@@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db *phy_db,
 }
 IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
 
-static int is_valid_channel(u16 ch_id)
+static bool is_valid_channel(u16 ch_id)
 {
-   if (ch_id <= 14 ||
-   (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
-   (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
-   (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
-   return 1;
-   return 0;
+   return (ch_id <= 14 ||
+  (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
+  (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
+  (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
 }
 
 static u8 ch_id_to_ch_index(u16 ch_id)
-- 
2.13.5

[PATCH 1/3] wireless: iwlwifi: use bool instead of int

[Christoph Böhmwalder]

Change a usage of int in a boolean context to use the bool type instead, as it
makes the intent of the function clearer and helps clarify its semantics.

Also eliminate the if/else and just return the boolean result directly,
making the code more readable.

Signed-off-by: Christoph Böhmwalder 
---
 drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
index b7cd813ba70f..0eb815ae97e8 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
@@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db *phy_db,
 }
 IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
 
-static int is_valid_channel(u16 ch_id)
+static bool is_valid_channel(u16 ch_id)
 {
-   if (ch_id <= 14 ||
-   (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
-   (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
-   (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
-   return 1;
-   return 0;
+   return (ch_id <= 14 ||
+  (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
+  (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
+  (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
 }
 
 static u8 ch_id_to_ch_index(u16 ch_id)
-- 
2.13.5

[PATCH 3/3] wireless: iwlwifi: wrap macro into braces

[Christoph Böhmwalder]

Macros should always be wrapped in braces, so fix this instance.

Signed-off-by: Christoph Böhmwalder <christ...@boehmwalder.at>
---
 drivers/net/wireless/intel/iwlwifi/iwl-io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-io.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-io.c
index efb1998dcabd..0211963b3e71 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-io.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-io.c
@@ -252,7 +252,7 @@ IWL_EXPORT_SYMBOL(iwl_force_nmi);
 
 static const char *get_rfh_string(int cmd)
 {
-#define IWL_CMD(x) case x: return #x
+#define IWL_CMD(x) { case x: return #x; }
 #define IWL_CMD_MQ(arg, reg, q) { if (arg == reg(q)) return #reg; }
 
int i;
-- 
2.13.5

[PATCH 3/3] wireless: iwlwifi: wrap macro into braces

[Christoph Böhmwalder]

Macros should always be wrapped in braces, so fix this instance.

Signed-off-by: Christoph Böhmwalder 
---
 drivers/net/wireless/intel/iwlwifi/iwl-io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-io.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-io.c
index efb1998dcabd..0211963b3e71 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-io.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-io.c
@@ -252,7 +252,7 @@ IWL_EXPORT_SYMBOL(iwl_force_nmi);
 
 static const char *get_rfh_string(int cmd)
 {
-#define IWL_CMD(x) case x: return #x
+#define IWL_CMD(x) { case x: return #x; }
 #define IWL_CMD_MQ(arg, reg, q) { if (arg == reg(q)) return #reg; }
 
int i;
-- 
2.13.5

[PATCH 0/3] iwlwifi: cosmetic fixes

[Christoph Böhmwalder]

Fix several code style issues, some of which were reported by checkpatch.pl.

The changes are:
* One instance of an `int` variable being used in a boolean context, chaned to
  use the more appropriate `bool` type.
* One very minor fix, removing a newline between a function definition and its
  associated `static` keyword
* One fix wrapping a macro in curly braces


Christoph Böhmwalder (3):
  wireless: iwlwifi: use bool instead of int
  wireless: iwlwifi: function definition cosmetic fix
  wireless: iwlwifi: wrap macro into braces

 drivers/net/wireless/intel/iwlwifi/iwl-io.c |  2 +-
 drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 16 +++-
 2 files changed, 8 insertions(+), 10 deletions(-)

-- 
2.13.5

[PATCH 0/3] iwlwifi: cosmetic fixes

[Christoph Böhmwalder]

Fix several code style issues, some of which were reported by checkpatch.pl.

The changes are:
* One instance of an `int` variable being used in a boolean context, chaned to
  use the more appropriate `bool` type.
* One very minor fix, removing a newline between a function definition and its
  associated `static` keyword
* One fix wrapping a macro in curly braces


Christoph Böhmwalder (3):
  wireless: iwlwifi: use bool instead of int
  wireless: iwlwifi: function definition cosmetic fix
  wireless: iwlwifi: wrap macro into braces

 drivers/net/wireless/intel/iwlwifi/iwl-io.c |  2 +-
 drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 16 +++-
 2 files changed, 8 insertions(+), 10 deletions(-)

-- 
2.13.5

Re: [PATCH 1/6] [media] tda8261: Use common error handling code in tda8261_set_params()

[Christoph Böhmwalder]

On 26.09.2017 13:27, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Tue, 26 Sep 2017 11:01:44 +0200
> 
> * Add a jump target so that a bit of exception handling can be better
>   reused at the end of this function.
> 
>   This issue was detected by using the Coccinelle software.
> 
> * The script "checkpatch.pl" pointed information out like the following.
> 
>   ERROR: do not use assignment in if condition
> 
>   Thus fix an affected source code place.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/media/dvb-frontends/tda8261.c | 20 
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/tda8261.c 
> b/drivers/media/dvb-frontends/tda8261.c
> index 4eb294f330bc..5a8a9b6b8107 100644
> --- a/drivers/media/dvb-frontends/tda8261.c
> +++ b/drivers/media/dvb-frontends/tda8261.c
> @@ -129,18 +129,18 @@ static int tda8261_set_params(struct dvb_frontend *fe)
>  
>   /* Set params */
>   err = tda8261_write(state, buf);
> - if (err < 0) {
> - pr_err("%s: I/O Error\n", __func__);
> - return err;
> - }
> + err = tda8261_get_status(fe, );
> + if (err < 0)
> + goto report_failure;
> +

Is this change really correct? Doesn't it query the status once more
often than before?

>   /* sleep for some time */
>   pr_debug("%s: Waiting to Phase LOCK\n", __func__);
>   msleep(20);
>   /* check status */
> - if ((err = tda8261_get_status(fe, )) < 0) {
> - pr_err("%s: I/O Error\n", __func__);
> - return err;
> - }
> + err = tda8261_get_status(fe, );
> + if (err < 0)
> + goto report_failure;
> +
>   if (status == 1) {
>   pr_debug("%s: Tuner Phase locked: status=%d\n", __func__,
>status);
> @@ -150,6 +150,10 @@ static int tda8261_set_params(struct dvb_frontend *fe)
>   }
>  
>   return 0;
> +
> +report_failure:
> + pr_err("%s: I/O Error\n", __func__);
> + return err;
>  }
>  
>  static void tda8261_release(struct dvb_frontend *fe)
> 


-- 
Regards,
Christoph



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/6] [media] tda8261: Use common error handling code in tda8261_set_params()

[Christoph Böhmwalder]

On 26.09.2017 13:27, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Tue, 26 Sep 2017 11:01:44 +0200
> 
> * Add a jump target so that a bit of exception handling can be better
>   reused at the end of this function.
> 
>   This issue was detected by using the Coccinelle software.
> 
> * The script "checkpatch.pl" pointed information out like the following.
> 
>   ERROR: do not use assignment in if condition
> 
>   Thus fix an affected source code place.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/media/dvb-frontends/tda8261.c | 20 
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/tda8261.c 
> b/drivers/media/dvb-frontends/tda8261.c
> index 4eb294f330bc..5a8a9b6b8107 100644
> --- a/drivers/media/dvb-frontends/tda8261.c
> +++ b/drivers/media/dvb-frontends/tda8261.c
> @@ -129,18 +129,18 @@ static int tda8261_set_params(struct dvb_frontend *fe)
>  
>   /* Set params */
>   err = tda8261_write(state, buf);
> - if (err < 0) {
> - pr_err("%s: I/O Error\n", __func__);
> - return err;
> - }
> + err = tda8261_get_status(fe, );
> + if (err < 0)
> + goto report_failure;
> +

Is this change really correct? Doesn't it query the status once more
often than before?

>   /* sleep for some time */
>   pr_debug("%s: Waiting to Phase LOCK\n", __func__);
>   msleep(20);
>   /* check status */
> - if ((err = tda8261_get_status(fe, )) < 0) {
> - pr_err("%s: I/O Error\n", __func__);
> - return err;
> - }
> + err = tda8261_get_status(fe, );
> + if (err < 0)
> + goto report_failure;
> +
>   if (status == 1) {
>   pr_debug("%s: Tuner Phase locked: status=%d\n", __func__,
>status);
> @@ -150,6 +150,10 @@ static int tda8261_set_params(struct dvb_frontend *fe)
>   }
>  
>   return 0;
> +
> +report_failure:
> + pr_err("%s: I/O Error\n", __func__);
> + return err;
>  }
>  
>  static void tda8261_release(struct dvb_frontend *fe)
> 


-- 
Regards,
Christoph



signature.asc
Description: OpenPGP digital signature

Re: [PATCH RESEND] wireless: iwlwifi: fix minor code style issues

[Christoph Böhmwalder]

> Why are you already resending this?

Sorry, I guess I was too impatient. I also messed up the spelling in a
"To:" line and forgot triv...@kernel.org the first time I sent it, so I
figured I'd just fix it in a resend.

I'll make sure to wait a little longer next time.

--
Regards,
Christoph



signature.asc
Description: OpenPGP digital signature

Re: [PATCH RESEND] wireless: iwlwifi: fix minor code style issues

[Christoph Böhmwalder]

> Why are you already resending this?

Sorry, I guess I was too impatient. I also messed up the spelling in a
"To:" line and forgot triv...@kernel.org the first time I sent it, so I
figured I'd just fix it in a resend.

I'll make sure to wait a little longer next time.

--
Regards,
Christoph



signature.asc
Description: OpenPGP digital signature

[PATCH RESEND] wireless: iwlwifi: fix minor code style issues

[Christoph Böhmwalder]

Fixes three trivial issues as reported by checkpatch.pl, namely two
switch/case indentation issues and one alignment issue in a multiline
comment.

Signed-off-by: Christoph Böhmwalder <christ...@boehmwalder.at>
---
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index 99676d6c4713..ccdb247d68c5 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -832,7 +832,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
capa->standard_phy_calibration_size =
le32_to_cpup((__le32 *)tlv_data);
break;
-case IWL_UCODE_TLV_SEC_RT:
+   case IWL_UCODE_TLV_SEC_RT:
iwl_store_ucode_sec(pieces, tlv_data, IWL_UCODE_REGULAR,
tlv_len);
drv->fw.type = IWL_FW_MVM;
@@ -864,7 +864,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
FW_PHY_CFG_RX_CHAIN) >>
FW_PHY_CFG_RX_CHAIN_POS;
break;
-case IWL_UCODE_TLV_SECURE_SEC_RT:
+   case IWL_UCODE_TLV_SECURE_SEC_RT:
iwl_store_ucode_sec(pieces, tlv_data, IWL_UCODE_REGULAR,
tlv_len);
drv->fw.type = IWL_FW_MVM;
@@ -1335,7 +1335,8 @@ static void iwl_req_fw_callback(const struct
firmware *ucode_raw, void *context)
/* Runtime instructions and 2 copies of data:
 * 1) unmodified from disk
-* 2) backup cache for save/restore during power-downs */
+* 2) backup cache for save/restore during power-downs
+*/
for (i = 0; i < IWL_UCODE_TYPE_MAX; i++)
if (iwl_alloc_ucode(drv, pieces, i))
goto out_free_fw;
-- 
2.13.5




signature.asc
Description: OpenPGP digital signature

[PATCH RESEND] wireless: iwlwifi: fix minor code style issues

[Christoph Böhmwalder]

Fixes three trivial issues as reported by checkpatch.pl, namely two
switch/case indentation issues and one alignment issue in a multiline
comment.

Signed-off-by: Christoph Böhmwalder 
---
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index 99676d6c4713..ccdb247d68c5 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -832,7 +832,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
capa->standard_phy_calibration_size =
le32_to_cpup((__le32 *)tlv_data);
break;
-case IWL_UCODE_TLV_SEC_RT:
+   case IWL_UCODE_TLV_SEC_RT:
iwl_store_ucode_sec(pieces, tlv_data, IWL_UCODE_REGULAR,
tlv_len);
drv->fw.type = IWL_FW_MVM;
@@ -864,7 +864,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
FW_PHY_CFG_RX_CHAIN) >>
FW_PHY_CFG_RX_CHAIN_POS;
break;
-case IWL_UCODE_TLV_SECURE_SEC_RT:
+   case IWL_UCODE_TLV_SECURE_SEC_RT:
iwl_store_ucode_sec(pieces, tlv_data, IWL_UCODE_REGULAR,
tlv_len);
drv->fw.type = IWL_FW_MVM;
@@ -1335,7 +1335,8 @@ static void iwl_req_fw_callback(const struct
firmware *ucode_raw, void *context)
/* Runtime instructions and 2 copies of data:
 * 1) unmodified from disk
-* 2) backup cache for save/restore during power-downs */
+* 2) backup cache for save/restore during power-downs
+*/
for (i = 0; i < IWL_UCODE_TYPE_MAX; i++)
if (iwl_alloc_ucode(drv, pieces, i))
goto out_free_fw;
-- 
2.13.5




signature.asc
Description: OpenPGP digital signature

Re: [PATCH] [trivial] Fix typos in printk

[Christoph Böhmwalder]

>   if ((stream_fs != 8000) && (stream_fs != 16000)) {
> - dev_err(afe->dev, "%s() btmgr not supprt this stream_fs %d\n",
> + dev_err(afe->dev, "%s() btmgr not support this stream_fs %d\n",

I might be mistaken, but shouldn't it be "[...] btmgr *does* not support
this stream_fs [...]" anyways?

>   __func__, stream_fs);
>   return -EINVAL;
>   }
>

--
Regards,
Christoph



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] [trivial] Fix typos in printk

[Christoph Böhmwalder]

>   if ((stream_fs != 8000) && (stream_fs != 16000)) {
> - dev_err(afe->dev, "%s() btmgr not supprt this stream_fs %d\n",
> + dev_err(afe->dev, "%s() btmgr not support this stream_fs %d\n",

I might be mistaken, but shouldn't it be "[...] btmgr *does* not support
this stream_fs [...]" anyways?

>   __func__, stream_fs);
>   return -EINVAL;
>   }
>

--
Regards,
Christoph



signature.asc
Description: OpenPGP digital signature

[PATCH] wireless: iwlwifi: fix minor code style issues

[Christoph Böhmwalder]

Fixes three trivial issues as reported by checkpatch.pl, namely two
switch/case indentation issues and one alignment issue in a multiline comment.

Signed-off-by: Christoph Böhmwalder <christ...@boehmwalder.at>
---
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index 99676d6c4713..ccdb247d68c5 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -832,7 +832,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
capa->standard_phy_calibration_size =
le32_to_cpup((__le32 *)tlv_data);
break;
-case IWL_UCODE_TLV_SEC_RT:
+   case IWL_UCODE_TLV_SEC_RT:
iwl_store_ucode_sec(pieces, tlv_data, IWL_UCODE_REGULAR,
tlv_len);
drv->fw.type = IWL_FW_MVM;
@@ -864,7 +864,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
FW_PHY_CFG_RX_CHAIN) >>
FW_PHY_CFG_RX_CHAIN_POS;
break;
-case IWL_UCODE_TLV_SECURE_SEC_RT:
+   case IWL_UCODE_TLV_SECURE_SEC_RT:
iwl_store_ucode_sec(pieces, tlv_data, IWL_UCODE_REGULAR,
tlv_len);
drv->fw.type = IWL_FW_MVM;
@@ -1335,7 +1335,8 @@ static void iwl_req_fw_callback(const struct firmware 
*ucode_raw, void *context)
 
/* Runtime instructions and 2 copies of data:
 * 1) unmodified from disk
-* 2) backup cache for save/restore during power-downs */
+* 2) backup cache for save/restore during power-downs
+*/
for (i = 0; i < IWL_UCODE_TYPE_MAX; i++)
if (iwl_alloc_ucode(drv, pieces, i))
goto out_free_fw;
-- 
2.13.5

[PATCH] wireless: iwlwifi: fix minor code style issues

[Christoph Böhmwalder]

Fixes three trivial issues as reported by checkpatch.pl, namely two
switch/case indentation issues and one alignment issue in a multiline comment.

Signed-off-by: Christoph Böhmwalder 
---
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index 99676d6c4713..ccdb247d68c5 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -832,7 +832,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
capa->standard_phy_calibration_size =
le32_to_cpup((__le32 *)tlv_data);
break;
-case IWL_UCODE_TLV_SEC_RT:
+   case IWL_UCODE_TLV_SEC_RT:
iwl_store_ucode_sec(pieces, tlv_data, IWL_UCODE_REGULAR,
tlv_len);
drv->fw.type = IWL_FW_MVM;
@@ -864,7 +864,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
FW_PHY_CFG_RX_CHAIN) >>
FW_PHY_CFG_RX_CHAIN_POS;
break;
-case IWL_UCODE_TLV_SECURE_SEC_RT:
+   case IWL_UCODE_TLV_SECURE_SEC_RT:
iwl_store_ucode_sec(pieces, tlv_data, IWL_UCODE_REGULAR,
tlv_len);
drv->fw.type = IWL_FW_MVM;
@@ -1335,7 +1335,8 @@ static void iwl_req_fw_callback(const struct firmware 
*ucode_raw, void *context)
 
/* Runtime instructions and 2 copies of data:
 * 1) unmodified from disk
-* 2) backup cache for save/restore during power-downs */
+* 2) backup cache for save/restore during power-downs
+*/
for (i = 0; i < IWL_UCODE_TYPE_MAX; i++)
if (iwl_alloc_ucode(drv, pieces, i))
goto out_free_fw;
-- 
2.13.5