Re: wl1251 & mac address & calibration data

2016-12-15 Thread Daniel Wagner

On 12/16/2016 03:03 AM, Luis R. Rodriguez wrote:

For the new API a solution for "fallback mechanisms" should be clean
though and I am looking to stay as far as possible from the existing
mess. A solution to help both the old API and new API is possible for
the "fallback mechanism" though -- but for that I can only refer you
at this point to some of Daniel Wagner and Tom Gunderson's firmwared
deamon prospect. It should help pave the way for a clean solution and
help address other stupid issues.


The firmwared project is hosted here

https://github.com/teg/firmwared

As Luis pointed out, firmwared relies on FW_LOADER_USER_HELPER_FALLBACK, 
which is not enabled by default. I don't see any reason why firmwared 
should not also support loading calibration data. If we find a sound way 
to do this.


As you can see from the commit history it is a pretty young project and 
more ore less reanimation of the old udev firmware loader feature.  We 
are getting int into shape, adding integration tests etc.


The main motivation for this project is the get movement back in stuck 
discussion on the firmware loader API. Luis was very busy writing up all 
the details on the current situation and purely from the amount of 
documentation need to describe the API you can tell something is awry.


Thanks,
Daniel


[PATCH 2/2] ath10k: use complete() instead complete_all()

2016-08-18 Thread Daniel Wagner
From: Daniel Wagner <daniel.wag...@bmw-carit.de>

There is only one waiter for the completion, therefore there
is no need to use complete_all(). Let's make that clear by
using complete() instead of complete_all().

The usage pattern of the completion is:

waiter context  waker context

scan.started


ath10k_start_scan()
  lockdep_assert_held(conf_mutex)
  auth10k_wmi_start_scan()
  wait_for_completion_timeout(scan.started)

ath10k_wmi_event_scan_start_failed()
  complete(scan.started)

ath10k_wmi_event_scan_started()
  complete(scan.started)

scan.completed
--

ath10k_scan_stop()
  lockdep_assert_held(conf_mutex)
  ath10k_wmi_stop_scan()
  wait_for_completion_timeout(scan.completed)

__ath10k_scan_finish()
  complete(scan.completed)

scan.on_channel
---

ath10k_remain_on_channel()
  mutex_lock(conf_mutex)
  ath10k_start_scan()
  wait_for_completion_timeout(scan.on_channel)

ath10k_wmi_event_scan_foreign_chan()
  complete(scan.on_channel)

offchan_tx_completed


ath10k_offchan_tx_work()
  mutex_lock(conf_mutex)
  reinit_completion(offchan_tx_completed)
  wait_for_completion_timeout(offchan_tx_completed)

ath10k_report_offchain_tx()
  complete(offchan_tx_completed)

install_key_done

ath10k_install_key()
  lockep_assert_held(conf_mutex)
  reinit_completion(install_key_done)
  wait_for_completion_timeout(install_key_done)

ath10k_htt_t2h_msg_handler()
  complete(install_key_done)

vdev_setup_done
---

ath10k_monitor_vdev_start()
  lockdep_assert_held(conf_mutex)
   reinit_completion(vdev_setup_done)
  ath10k_vdev_setup_sync()
wait_for_completion_timeout(vdev_setup_done)

ath10k_wmi_event_vdev_start_resp()
  complete(vdev_setup_done)

ath10k_monitor_vdev_stop()
  lockdep_assert_held(conf_mutex)
  reinit_completion(vdev_setup_done()
  ath10k_vdev_setup_sync()
wait_for_completion_timeout(vdev_setup_done)

ath10k_wmi_event_vdev_stopped()
 complete(vdev_setup_done)

thermal.wmi_sync

ath10k_thermal_show_temp()
  mutex_lock(conf_mutex)
  reinit_completion(thermal.wmi_sync)
  wait_for_completion_timeout(thermal.wmi_sync)

ath10k_thermal_event_temperature()
  complete(thermal.wmi_sync)

bss_survey_done
---
ath10k_mac_update_bss_chan_survey
  lockdep_assert_held(conf_mutex)
  reinit_completion(bss_survey_done)
  wait_for_completion_timeout(bss_survey_done)

ath10k_wmi_event_pdev_bss_chan_info()
  complete(bss_survey_done)

All complete() calls happen while the conf_mutex is taken. That means
at max one waiter is possible.

Signed-off-by: Daniel Wagner <daniel.wag...@bmw-carit.de>
---
 drivers/net/wireless/ath/ath10k/core.c | 16 
 drivers/net/wireless/ath/ath10k/mac.c  |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index e889829..ed76601 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1497,14 +1497,14 @@ static void ath10k_core_restart(struct work_struct 
*work)
 
ieee80211_stop_queues(ar->hw);
ath10k_drain_tx(ar);
-   complete_all(>scan.started);
-   complete_all(>scan.completed);
-   complete_all(>scan.on_channel);
-   complete_all(>offchan_tx_completed);
-   complete_all(>install_key_done);
-   complete_all(>vdev_setup_done);
-   complete_all(>thermal.wmi_sync);
-   complete_all(>bss_survey_done);
+   complete(>scan.started);
+   complete(>scan.completed);
+   complete(>scan.on_channel);
+   complete(>offchan_tx_completed);
+   complete(>install_key_done);
+   complete(>vdev_setup_done);
+   complete(>thermal.wmi_sync);
+   complete(>bss_survey_done);
wake_up(>htt.empty_tx_wq);
wake_up(>wmi.tx_credits_wq);
wake_up(>peer_mapping_wq);
diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
b/drivers/net/wireless/ath/ath10k/mac.c
index 0bbd0a0..c3c1c25 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k

[PATCH 0/2] wireless: Use complete() instead complete_all()

2016-08-18 Thread Daniel Wagner
From: Daniel Wagner <daniel.wag...@bmw-carit.de>

Hi,

Using complete_all() is not wrong per se but it suggest that there
might be more than one reader. For -rt I am reviewing all
complete_all() users and would like to leave only the real ones in the
tree. The main problem for -rt about complete_all() is that it can be
uses inside IRQ context and that can lead to unbounded amount work
inside the interrupt handler. That is a no no for -rt.

The patches grouped per subsystem and in small batches to allow
reviewing.

This series ignores all complete_all() usages in the firmware loading
path. They will be hopefully address by Luis' sysdata patches [0].
That leaves a couple of complete_all() calls.

The first patch fixes a real glitch for the carl9170 driver. I was
able to test it because I have the hardware. For the second one I
haven't found any dongle with that chip in my drawers. 

This series against net-next of today.

cheers,
daniel

[0] 
https://lkml.kernel.org/r/1466117661-22075-1-git-send-email-mcg...@kernel.org

Daniel Wagner (2):
  carl9170: Fix wrong completion usage
  ath10k: use complete() instead complete_all()

 drivers/net/wireless/ath/ath10k/core.c  | 16 
 drivers/net/wireless/ath/ath10k/mac.c   |  2 +-
 drivers/net/wireless/ath/carl9170/usb.c |  6 ++
 3 files changed, 11 insertions(+), 13 deletions(-)

-- 
2.7.4


[PATCH 1/2] carl9170: Fix wrong completion usage

2016-08-18 Thread Daniel Wagner
From: Daniel Wagner <daniel.wag...@bmw-carit.de>

carl9170_usb_stop() is used from several places to flush and cleanup any
pending work. The normal pattern is to send a request and wait for the
irq handler to call complete(). The completion is not reinitialized
during normal operation and as the old comment indicates it is important
to keep calls to wait_for_completion_timeout() and complete() balanced.

Calling complete_all() brings this equilibirum out of balance and needs
to be fixed by a reinit_completion(). But that opens a small race
window. It is possible that the sequence of complete_all(),
reinit_completion() is faster than the wait_for_completion_timeout() can
do its work. The wake up is not lost but the done counter test is after
reinit_completion() has been executed. The only reason we don't see
carl9170_exec_cmd() hang forever is we use the timeout version of
wait_for_copletion().

Let's fix this by reinitializing the completion (that is just setting
done counter to 0) just before we send out an request. Now,
carl9170_usb_stop() can be sure a complete() call is enough to make
progess since there is only one waiter at max. This is a common pattern
also seen in various drivers which use completion.

Signed-off-by: Daniel Wagner <daniel.wag...@bmw-carit.de>
---
 drivers/net/wireless/ath/carl9170/usb.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/usb.c 
b/drivers/net/wireless/ath/carl9170/usb.c
index 76842e6..99ab203 100644
--- a/drivers/net/wireless/ath/carl9170/usb.c
+++ b/drivers/net/wireless/ath/carl9170/usb.c
@@ -670,6 +670,7 @@ int carl9170_exec_cmd(struct ar9170 *ar, const enum 
carl9170_cmd_oids cmd,
ar->readlen = outlen;
spin_unlock_bh(>cmd_lock);
 
+   reinit_completion(>cmd_wait);
err = __carl9170_exec_cmd(ar, >cmd, false);
 
if (!(cmd & CARL9170_CMD_ASYNC_FLAG)) {
@@ -778,10 +779,7 @@ void carl9170_usb_stop(struct ar9170 *ar)
spin_lock_bh(>cmd_lock);
ar->readlen = 0;
spin_unlock_bh(>cmd_lock);
-   complete_all(>cmd_wait);
-
-   /* This is required to prevent an early completion on _start */
-   reinit_completion(>cmd_wait);
+   complete(>cmd_wait);
 
/*
 * Note:
-- 
2.7.4


Re: [RFC v0 7/8] Input: ims-pcu: use firmware_stat instead of completion

2016-08-03 Thread Daniel Wagner

On 08/02/2016 09:41 AM, Luis R. Rodriguez wrote:

On Tue, Aug 02, 2016 at 08:53:55AM +0200, Daniel Wagner wrote:

On 08/02/2016 08:34 AM, Luis R. Rodriguez wrote:

On Tue, Aug 02, 2016 at 07:49:19AM +0200, Daniel Wagner wrote:

So you argue for the remoteproc use case with 100+ MB firmware that
if there is a way to load after pivot_root() (or other additional
firmware partition shows up) then there is no need at all for
usermode helper?


No, I'm saying I'd like to hear valid uses cases for the usermode helper and so
far I have only found using coccinelle grammar 2 explicit users, that's it. My
patch series (not yet merge) then annotates these as valid as I've verified
through their documentation they have some quirky requirement.


I got that question wrong. It should read something like 'for the 
remoteproc 100+MB there is no need for the user help?'. I've gone 
through your patches and they make perfectly sense too. Maybe I can 
convince you to take a better version of my patch 3 into your queue. And 
I help you converting the exiting drivers. Obviously if you like my help 
at all.



Other than these two drivers I'd like hear to valid requirements for it.

The existential issue is a real issue but it does not look impossible to
resolve. It may be a solution to bloat up the kernel with 100+ MB size just to
stuff built-in firmware to avoid this issue, but it does not mean a solution
is not possible.

Remind me -- why can remoteproc not stuff the firmware in initramfs ?


I don't know. I was just bringing it up with the hope that Bjorn will 
defend it. It seems my tactics didn't work out :)



Anyway, here's a simple suggestion: fs/exec.c gets a sentinel file monitor
support per enum kernel_read_file_id. For instance we'd have one for
READING_FIRMWARE, one for READING_KEXEC_IMAGE, perhaps READING_POLICY, and this
would in turn be used as the system configurable deterministic file for
which to wait for to be present before enabling each enum kernel_read_file_id
type read.

Thoughts ?


Not sure if I get you here correctly. Is the 'system configurable 
deterministic file' is a knob which controlled by user space? Or it this 
something you define at compile time?


Hmm, so it would allow to decided to ask a userspace helper or load the 
firmware directly (to be more precised the kernel_read_file_id type). If 
yes, than it is what currently already have just integrated nicely into 
the new sysdata API.


cheers,
daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v0 7/8] Input: ims-pcu: use firmware_stat instead of completion

2016-08-02 Thread Daniel Wagner

On 08/02/2016 08:34 AM, Luis R. Rodriguez wrote:

On Tue, Aug 02, 2016 at 07:49:19AM +0200, Daniel Wagner wrote:

The sysdata API's main goal rather is to provide a flexible API first,
compartamentalizing the usermode helper was secondary. But now it seems
I may just also add devm support too to help simplify code further.


I missed the point that you plan to add usermode helper support to
the sysdata API.


I had no such plans, when I have asked folks so far about "hey are you
really in need for it, OK what for? " and "what extended uses do you
envision?" so I far I have not gotten any replies at all. So -- instead
sysdata currently ignores it.


So you argue for the remoteproc use case with 100+ MB firmware that if 
there is a way to load after pivot_root() (or other additional firmware 
partition shows up) then there is no need at all for usermode helper?

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v0 7/8] Input: ims-pcu: use firmware_stat instead of completion

2016-08-01 Thread Daniel Wagner

Hi Luis,


I was ignorant on all the nasty details around the firmware loading.
If I parse Luis' patches correctly they introduce an API which calls
kernel_read_file_from_path() asynchronously:

sysdata_file_request_async(..., )
  *coookie = async_schedule_domain(request_sysdata_file_work_func(), ..)

  request_sysdata_file_work_fun()
_sysdata_file_request()
  fw_get_filesystem_firmware()
kernel_read_file_from_path()

sysdata_synchronize_request();

Doesn't look like what your asking for.


No, but its also a generic kernel read issue as I noted in my last
reply.


Okay, got it.


If we want to overhaul firmware loading support we need to figure
out  how to support case when a driver want to [asynchronously] request
firmware/config/blob and the rest of the system is not ready. Even if we
want kernel to do read/load the data we need userspace to tell kernel
when firmware partition is available, until then the kernel should not
fail the request.


I gather from Luis' blog post and comments that he is on the quest
on removing userspace support completely.


No, I explained in my last proposed documentation patch series that we cannot
get rid of the usermode helper.


I stand corrected.


Its not well understood why so I explained and documented why.


Obviously, I got lost somewhere there :)


Best we can do is compartamentalize its uses.


Sounds like a plan.


The sysdata API's main goal rather is to provide a flexible API first,
compartamentalizing the usermode helper was secondary. But now it seems
I may just also add devm support too to help simplify code further.


I missed the point that you plan to add usermode helper support to the 
sysdata API.



What Dmitry notes is an existential issue with kernel_read_file_from_path()
and we need a common solution for it.


Understood. I guess best thing to keep that discussion in the other 
subthread.



Maybe this attempt here could be a step before. Step 1 would be
changing request_firmware_nowait() to request_firmware_async so
drivers don't have to come up with their own sync primitives, e.g.

  cookie = request_firmware_async()
  fw_load_wait(cookie)


That's one of the features already part of async mechanism of the sysdata API :)


Yes, I realized that too :)

cheers,
daniel

Thanks for the feedback.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v0 7/8] Input: ims-pcu: use firmware_stat instead of completion

2016-08-01 Thread Daniel Wagner

On 07/31/2016 09:23 AM, Dmitry Torokhov wrote:

On July 30, 2016 9:58:17 AM PDT, "Luis R. Rodriguez" <mcg...@kernel.org> wrote:

On Sat, Jul 30, 2016 at 02:42:41PM +0200, Arend van Spriel wrote:

On 29-07-16 08:13, Daniel Wagner wrote:

On 07/28/2016 09:01 PM, Bjorn Andersson wrote:

On Thu 28 Jul 11:33 PDT 2016, Dmitry Torokhov wrote:



+ Luis (again) ;-)


That was not on purpose :) My attempt to keep the Cc list a bit shorter 
was a failure.



Do not quite like it... I'd rather asynchronous request give out
firmware status pointer that could be used later on.


Excellent. Why not get rid of the callback function as well and have
fw_loading_wait() return result (0 = firmware available, < 0 = fail).
Just to confirm, you are proposing a new API function next to
request_firmware_nowait(), right?


If proposing new firmware_class patches please bounce / Cc me, I've
recently asked for me to be added to MAINTAINERS so I get these
e-mails as I'm working on a new flexible API which would allow us
to extend the firmware API without having to care about the old
stupid usermode helper at all.


These patches here are a first attempt to clean up a bit of the code 
around the completion API. As Dmitry correctly pointed out, it makes 
more sense to go bit further and make the async loading a bit more 
convenient for the drivers.



I am not sure why we started calling usermode helper "stupid". We
only  had to implement direct kernel firmware loading because udev/stsremd
folks had "interesting" ideas how events should be handled; but having
userspace to feed us data is not stupid.


I was ignorant on all the nasty details around the firmware loading. If 
I parse Luis' patches correctly they introduce an API which calls 
kernel_read_file_from_path() asynchronously:


sysdata_file_request_async(..., )
  *coookie = async_schedule_domain(request_sysdata_file_work_func(), ..)

  request_sysdata_file_work_fun()
_sysdata_file_request()
  fw_get_filesystem_firmware()
kernel_read_file_from_path()

sysdata_synchronize_request();

Doesn't look like what your asking for.


If we want to overhaul firmware loading support we need to figure
out  how to support case when a driver want to [asynchronously] request
firmware/config/blob and the rest of the system is not ready. Even if we
want kernel to do read/load the data we need userspace to tell kernel
when firmware partition is available, until then the kernel should not
fail the request.


I gather from Luis' blog post and comments that he is on the quest on 
removing userspace support completely.


Maybe this attempt here could be a step before. Step 1 would be changing 
request_firmware_nowait() to request_firmware_async so drivers don't 
have to come up with their own sync primitives, e.g.


  cookie = request_firmware_async()
  fw_load_wait(cookie)

Step 2 could be something more towards sysdata approach.

cheers,
daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v0 7/8] Input: ims-pcu: use firmware_stat instead of completion

2016-07-29 Thread Daniel Wagner

On 07/28/2016 09:01 PM, Bjorn Andersson wrote:

On Thu 28 Jul 11:33 PDT 2016, Dmitry Torokhov wrote:


On Thu, Jul 28, 2016 at 09:55:11AM +0200, Daniel Wagner wrote:

From: Daniel Wagner <daniel.wag...@bmw-carit.de>


[..]


Do not quite like it... I'd rather asynchronous request give out a
firmware status pointer that could be used later on.

pcu->fw_st = request_firmware_async(IMS_PCU_FIRMWARE_NAME,
pcu,
ims_pcu_process_async_firmware);
if (IS_ERR(pcu->fw_st))
return PTR_ERR(pcu->fw_st);



fw_loading_wait(pcu->fw_st);



In the remoteproc case (patch 6) this would clean up the code, rather
than replacing the completion API 1 to 1. I like it!


IIRC most drivers do it the same way. So request_firmware_async() indeed 
would be good thing to have. Let me try that.


Thanks for the excellent feedback.

cheers,
daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v0 3/8] firmware: Factor out firmware load helpers

2016-07-29 Thread Daniel Wagner

On 07/28/2016 07:57 PM, Dmitry Torokhov wrote:

On Thu, Jul 28, 2016 at 09:55:07AM +0200, Daniel Wagner wrote:

+int __firmware_stat_wait(struct firmware_stat *fwst,
+   long timeout)
+{
+   int err;
+   err = swait_event_interruptible_timeout(fwst->wq,
+   is_fw_sync_done(READ_ONCE(fwst->status)),
+   timeout);
+   if (err == 0 && fwst->status == FW_STATUS_ABORT)
+   return -ENOENT;
+
+   return err;
+}
+EXPORT_SYMBOL(__firmware_stat_wait);
+
+void __firmware_stat_set(struct firmware_stat *fwst, unsigned long status)
+{
+   WRITE_ONCE(fwst->status, status);
+   swake_up(>wq);


Do we need to notify everyone for FW_STATUS_LOADING status?


Hmm, I don't think so. In the end drivers are probably only interested 
in the final result which is either success or fail.


cheers,
daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v0 3/8] firmware: Factor out firmware load helpers

2016-07-29 Thread Daniel Wagner

It's somewhat odd to me that the structure is "firmware_stat" but most
of the functions are "fw_loading_*".  That seems inconsistent for a
structure that is (currently) only used by these functions.


I agree, my proposal is odd.


I would personally do either:

a) "struct fw_load_status" and "fw_load_*()"

or

b) "struct firmware_load_stat" and "firmware_load_*()"

I'd also change the function names from "loading" -> "load", similar to
how userland has read(2), not reading(2).


a) sounds good to me, because fw_load_ should be long enough prefix.

cheers,
daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v0 4/8] Input: goodix: use firmware_stat instead of completion

2016-07-28 Thread Daniel Wagner

Looking at the API, I really don't like the mixing of namespaces.
Either it's fw_ or it's firmware_ but not a mix of both.


I agree, that was not really clever.

struct fw_loading { ... }
__fw_loading_*()
fw_loading_*()

Would that make more sense? firmware_loading_ as prefix is a bit long IMO.


Also looks like fw_loading_start() would do nothing as the struct is
likely zero initialised, and FW_STATUS_LOADING == 0. Maybe you need an
UNSET enum member?


Good point, I cut a corner here a bit :). In the spirit of making things 
more clear fw_loading_start() should be used.



FW_STATUS_ABORT <- FW_STATUS_ABORTED, to match the adjective suffixes
of the other members. Ditto fw_loading_abort() which doesn't abort
firmware loading but sets the status.


Okay.

Thanks a lot for the feedback.

cheers,
daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v0 4/8] Input: goodix: use firmware_stat instead of completion

2016-07-28 Thread Daniel Wagner

On 07/28/2016 01:22 PM, Bastien Nocera wrote:

On Thu, 2016-07-28 at 09:55 +0200, Daniel Wagner wrote:

From: Daniel Wagner <daniel.wag...@bmw-carit.de>

Loading firmware is an operation many drivers implement in various
ways
around the completion API. And most of them do it almost in the same
way. Let's reuse the firmware_stat API which is used also by the
firmware_class loader. Apart of streamlining the firmware loading
states
we also document it slightly better.

Signed-off-by: Daniel Wagner <daniel.wag...@bmw-carit.de>


Irina added and tested that feature, so best for her to comment on
this, as I don't have any hardware that would use this feature.


In case you have any comments on the API, let me know. I'll add Irina to 
the Cc list in the next version.


cheers,
daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC v0 2/8] selftests: firmware: do not clutter output

2016-07-28 Thread Daniel Wagner
From: Daniel Wagner <daniel.wag...@bmw-carit.de>

Some of the test are supposed to fail but we get a few ouptput lines:

./fw_filesystem.sh: line 51: printf: write error: Invalid argument
./fw_filesystem.sh: line 56: printf: write error: No such device
./fw_filesystem.sh: line 62: echo: write error: No such file or directory

Let's silence them so that the selftest output is clean if all is fine.

Signed-off-by: Daniel Wagner <daniel.wag...@bmw-carit.de>
---
 tools/testing/selftests/firmware/fw_filesystem.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh 
b/tools/testing/selftests/firmware/fw_filesystem.sh
index 5c495ad..d8ac9ba 100755
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -48,18 +48,18 @@ echo "ABCD0123" >"$FW"
 
 NAME=$(basename "$FW")
 
-if printf '\000' >"$DIR"/trigger_request; then
+if printf '\000' >"$DIR"/trigger_request 2> /dev/null; then
echo "$0: empty filename should not succeed" >&2
exit 1
 fi
 
-if printf '\000' >"$DIR"/trigger_async_request; then
+if printf '\000' >"$DIR"/trigger_async_request 2> /dev/null; then
echo "$0: empty filename should not succeed (async)" >&2
exit 1
 fi
 
 # Request a firmware that doesn't exist, it should fail.
-if echo -n "nope-$NAME" >"$DIR"/trigger_request; then
+if echo -n "nope-$NAME" >"$DIR"/trigger_request 2> /dev/null; then
echo "$0: firmware shouldn't have loaded" >&2
exit 1
 fi
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC v0 5/8] ath9k_htc: use firmware_stat instead of completion

2016-07-28 Thread Daniel Wagner
From: Daniel Wagner <daniel.wag...@bmw-carit.de>

Loading firmware is an operation many drivers implement in various ways
around the completion API. And most of them do it almost in the same
way. Let's reuse the firmware_stat API which is used also by the
firmware_class loader. Apart of streamlining the firmware loading states
we also document it slightly better.

Signed-off-by: Daniel Wagner <daniel.wag...@bmw-carit.de>
---
 drivers/net/wireless/ath/ath9k/hif_usb.c | 10 +-
 drivers/net/wireless/ath/ath9k/hif_usb.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c 
b/drivers/net/wireless/ath/ath9k/hif_usb.c
index e1c338c..0a05d68 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -1067,7 +1067,7 @@ static void ath9k_hif_usb_firmware_fail(struct 
hif_device_usb *hif_dev)
struct device *dev = _dev->udev->dev;
struct device *parent = dev->parent;
 
-   complete_all(_dev->fw_done);
+   fw_loading_abort(hif_dev->fw_st);
 
if (parent)
device_lock(parent);
@@ -1192,7 +1192,7 @@ static void ath9k_hif_usb_firmware_cb(const struct 
firmware *fw, void *context)
 
release_firmware(fw);
hif_dev->flags |= HIF_USB_READY;
-   complete_all(_dev->fw_done);
+   fw_loading_done(hif_dev->fw_st);
 
return;
 
@@ -1287,7 +1287,7 @@ static int ath9k_hif_usb_probe(struct usb_interface 
*interface,
 #endif
usb_set_intfdata(interface, hif_dev);
 
-   init_completion(_dev->fw_done);
+   firmware_stat_init(_dev->fw_st);
 
ret = ath9k_hif_request_firmware(hif_dev, true);
if (ret)
@@ -1330,7 +1330,7 @@ static void ath9k_hif_usb_disconnect(struct usb_interface 
*interface)
if (!hif_dev)
return;
 
-   wait_for_completion(_dev->fw_done);
+   fw_loading_wait(hif_dev->fw_st);
 
if (hif_dev->flags & HIF_USB_READY) {
ath9k_htc_hw_deinit(hif_dev->htc_handle, unplugged);
@@ -1363,7 +1363,7 @@ static int ath9k_hif_usb_suspend(struct usb_interface 
*interface,
if (!(hif_dev->flags & HIF_USB_START))
ath9k_htc_suspend(hif_dev->htc_handle);
 
-   wait_for_completion(_dev->fw_done);
+   fw_loading_wait(hif_dev->fw_st);
 
if (hif_dev->flags & HIF_USB_READY)
ath9k_hif_usb_dealloc_urbs(hif_dev);
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h 
b/drivers/net/wireless/ath/ath9k/hif_usb.h
index 7c2ef7e..0af9fe4 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.h
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
@@ -111,7 +111,7 @@ struct hif_device_usb {
const struct usb_device_id *usb_device_id;
const void *fw_data;
size_t fw_size;
-   struct completion fw_done;
+   struct firmware_stat fw_st;
struct htc_target *htc_handle;
struct hif_usb_tx tx;
struct usb_anchor regout_submitted;
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC v0 8/8] iwl4965: use firmware_stat instead of completion

2016-07-28 Thread Daniel Wagner
From: Daniel Wagner <daniel.wag...@bmw-carit.de>

Loading firmware is an operation many drivers implement in various ways
around the completion API. And most of them do it almost in the same
way. Let's reuse the firmware_stat API which is used also by the
firmware_class loader. Apart of streamlining the firmware loading states
we also document it slightly better.

Signed-off-by: Daniel Wagner <daniel.wag...@bmw-carit.de>
---
 drivers/net/wireless/intel/iwlegacy/4965-mac.c | 8 
 drivers/net/wireless/intel/iwlegacy/common.h   | 3 ++-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlegacy/4965-mac.c 
b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
index a91d170..d5e5808 100644
--- a/drivers/net/wireless/intel/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
@@ -5005,7 +5005,7 @@ il4965_ucode_callback(const struct firmware *ucode_raw, 
void *context)
 
/* We have our copies now, allow OS release its copies */
release_firmware(ucode_raw);
-   complete(>_4965.firmware_loading_complete);
+   fw_loading_done(il->_4965.fw_st);
return;
 
 try_again:
@@ -5019,7 +5019,7 @@ err_pci_alloc:
IL_ERR("failed to allocate pci memory\n");
il4965_dealloc_ucode_pci(il);
 out_unbind:
-   complete(>_4965.firmware_loading_complete);
+   fw_loading_done(il->_4965.fw_st);
device_release_driver(>pci_dev->dev);
release_firmware(ucode_raw);
 }
@@ -6678,7 +6678,7 @@ il4965_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
 
il_power_initialize(il);
 
-   init_completion(>_4965.firmware_loading_complete);
+   firmware_stat_init(>_4965.fw_st);
 
err = il4965_request_firmware(il, true);
if (err)
@@ -6716,7 +6716,7 @@ il4965_pci_remove(struct pci_dev *pdev)
if (!il)
return;
 
-   wait_for_completion(>_4965.firmware_loading_complete);
+   fw_loading_wait(il->_4965.fw_st);
 
D_INFO("*** UNLOAD DRIVER ***\n");
 
diff --git a/drivers/net/wireless/intel/iwlegacy/common.h 
b/drivers/net/wireless/intel/iwlegacy/common.h
index 726ede3..94af7b7 100644
--- a/drivers/net/wireless/intel/iwlegacy/common.h
+++ b/drivers/net/wireless/intel/iwlegacy/common.h
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -1357,7 +1358,7 @@ struct il_priv {
bool last_phy_res_valid;
u32 ampdu_ref;
 
-   struct completion firmware_loading_complete;
+   struct firmware_stat fw_st;
 
/*
 * chain noise reset and gain commands are the
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC v0 6/8] remoteproc: use firmware_stat instead of completion

2016-07-28 Thread Daniel Wagner
From: Daniel Wagner <daniel.wag...@bmw-carit.de>

Loading firmware is an operation many drivers implement in various ways
around the completion API. And most of them do it almost in the same
way. Let's reuse the firmware_stat API which is used also by the
firmware_class loader. Apart of streamlining the firmware loading states
we also document it slightly better.

Signed-off-by: Daniel Wagner <daniel.wag...@bmw-carit.de>
---
 drivers/remoteproc/remoteproc_core.c | 10 +-
 drivers/soc/ti/wkup_m3_ipc.c |  2 +-
 include/linux/remoteproc.h   |  6 --
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c 
b/drivers/remoteproc/remoteproc_core.c
index db3958b..3b158f7 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -936,7 +936,7 @@ static void rproc_fw_config_virtio(const struct firmware 
*fw, void *context)
 out:
release_firmware(fw);
/* allow rproc_del() contexts, if any, to proceed */
-   complete_all(>firmware_loading_complete);
+   fw_loading_done(rproc->fw_st);
 }
 
 static int rproc_add_virtio_devices(struct rproc *rproc)
@@ -944,7 +944,7 @@ static int rproc_add_virtio_devices(struct rproc *rproc)
int ret;
 
/* rproc_del() calls must wait until async loader completes */
-   init_completion(>firmware_loading_complete);
+   firmware_stat_init(>fw_st);
 
/*
 * We must retrieve early virtio configuration info from
@@ -959,7 +959,7 @@ static int rproc_add_virtio_devices(struct rproc *rproc)
  rproc, rproc_fw_config_virtio);
if (ret < 0) {
dev_err(>dev, "request_firmware_nowait err: %d\n", ret);
-   complete_all(>firmware_loading_complete);
+   fw_loading_abort(rproc->fw_st);
}
 
return ret;
@@ -1089,7 +1089,7 @@ static int __rproc_boot(struct rproc *rproc, bool wait)
 
/* if rproc virtio is not yet configured, wait */
if (wait)
-   wait_for_completion(>firmware_loading_complete);
+   fw_loading_wait(rproc->fw_st);
 
ret = rproc_fw_boot(rproc, firmware_p);
 
@@ -1447,7 +1447,7 @@ int rproc_del(struct rproc *rproc)
return -EINVAL;
 
/* if rproc is just being registered, wait */
-   wait_for_completion(>firmware_loading_complete);
+   fw_loading_wait(rproc->fw_st);
 
/* clean up remote vdev entries */
list_for_each_entry_safe(rvdev, tmp, >rvdevs, node)
diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c
index 8823cc8..14c6396 100644
--- a/drivers/soc/ti/wkup_m3_ipc.c
+++ b/drivers/soc/ti/wkup_m3_ipc.c
@@ -370,7 +370,7 @@ static void wkup_m3_rproc_boot_thread(struct wkup_m3_ipc 
*m3_ipc)
struct device *dev = m3_ipc->dev;
int ret;
 
-   wait_for_completion(_ipc->rproc->firmware_loading_complete);
+   fw_loading_wait(m3_ipc->rproc->fw_st);
 
init_completion(_ipc->sync_complete);
 
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 1c457a8..b8e7ff4 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /**
  * struct resource_table - firmware resource table header
@@ -397,7 +398,7 @@ enum rproc_crash_type {
  * @num_traces: number of trace buffers
  * @carveouts: list of physically contiguous memory allocations
  * @mappings: list of iommu mappings we initiated, needed on shutdown
- * @firmware_loading_complete: marks e/o asynchronous firmware loading
+ * @fw_sync: marks e/o asynchronous firmware loading
  * @bootaddr: address of first instruction to boot rproc with (optional)
  * @rvdevs: list of remote virtio devices
  * @notifyids: idr for dynamically assigning rproc-wide unique notify ids
@@ -429,7 +430,7 @@ struct rproc {
int num_traces;
struct list_head carveouts;
struct list_head mappings;
-   struct completion firmware_loading_complete;
+   struct firmware_stat fw_st;
u32 bootaddr;
struct list_head rvdevs;
struct idr notifyids;
@@ -479,6 +480,7 @@ struct rproc_vring {
  * @vring: the vrings for this vdev
  * @rsc_offset: offset of the vdev's resource entry
  */
+
 struct rproc_vdev {
struct list_head node;
struct rproc *rproc;
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC v0 3/8] firmware: Factor out firmware load helpers

2016-07-28 Thread Daniel Wagner
From: Daniel Wagner <daniel.wag...@bmw-carit.de>

Factor out the firmware loading synchronization code in order to allow
drivers to reuse it. This also documents more clearly what is
happening. This is especial useful the drivers which will be converted
afterwards. Not everyone has to come with yet another way to handle it.

We use swait instead completion. complete() would only wake up one
waiter, so complete_all() is used. complete_all() wakes max MAX_UINT/2
waiters which is plenty in all cases. Though withh swait we just wait
until the exptected status shows up and wake any waiter.

Signed-off-by: Daniel Wagner <daniel.wag...@bmw-carit.de>
---
 drivers/base/firmware_class.c | 112 +++---
 include/linux/firmware.h  |  71 ++
 2 files changed, 122 insertions(+), 61 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 773fc30..bf1ca70 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -85,11 +86,36 @@ static inline bool fw_is_builtin_firmware(const struct 
firmware *fw)
 }
 #endif
 
-enum {
-   FW_STATUS_LOADING,
-   FW_STATUS_DONE,
-   FW_STATUS_ABORT,
-};
+
+#if defined(CONFIG_FW_LOADER) || (defined(CONFIG_FW_LOADER_MODULE) && 
defined(MODULE))
+
+static inline bool is_fw_sync_done(unsigned long status)
+{
+   return status == FW_STATUS_LOADED || status == FW_STATUS_ABORT;
+}
+
+int __firmware_stat_wait(struct firmware_stat *fwst,
+   long timeout)
+{
+   int err;
+   err = swait_event_interruptible_timeout(fwst->wq,
+   is_fw_sync_done(READ_ONCE(fwst->status)),
+   timeout);
+   if (err == 0 && fwst->status == FW_STATUS_ABORT)
+   return -ENOENT;
+
+   return err;
+}
+EXPORT_SYMBOL(__firmware_stat_wait);
+
+void __firmware_stat_set(struct firmware_stat *fwst, unsigned long status)
+{
+   WRITE_ONCE(fwst->status, status);
+   swake_up(>wq);
+}
+EXPORT_SYMBOL(__firmware_stat_set);
+
+#endif
 
 static int loading_timeout = 60;   /* In seconds */
 
@@ -138,9 +164,8 @@ struct firmware_cache {
 struct firmware_buf {
struct kref ref;
struct list_head list;
-   struct completion completion;
+   struct firmware_stat fwst;
struct firmware_cache *fwc;
-   unsigned long status;
void *data;
size_t size;
 #ifdef CONFIG_FW_LOADER_USER_HELPER
@@ -194,7 +219,7 @@ static struct firmware_buf *__allocate_fw_buf(const char 
*fw_name,
 
kref_init(>ref);
buf->fwc = fwc;
-   init_completion(>completion);
+   firmware_stat_init(>fwst);
 #ifdef CONFIG_FW_LOADER_USER_HELPER
INIT_LIST_HEAD(>pending_list);
 #endif
@@ -292,15 +317,6 @@ static const char * const fw_path[] = {
 module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
 MODULE_PARM_DESC(path, "customized firmware image search path with a higher 
priority than default path");
 
-static void fw_finish_direct_load(struct device *device,
- struct firmware_buf *buf)
-{
-   mutex_lock(_lock);
-   set_bit(FW_STATUS_DONE, >status);
-   complete_all(>completion);
-   mutex_unlock(_lock);
-}
-
 static int fw_get_filesystem_firmware(struct device *device,
   struct firmware_buf *buf)
 {
@@ -339,7 +355,7 @@ static int fw_get_filesystem_firmware(struct device *device,
}
dev_dbg(device, "direct-loading %s\n", buf->fw_id);
buf->size = size;
-   fw_finish_direct_load(device, buf);
+   fw_loading_done(buf->fwst);
break;
}
__putname(path);
@@ -457,12 +473,11 @@ static void __fw_load_abort(struct firmware_buf *buf)
 * There is a small window in which user can write to 'loading'
 * between loading done and disappearance of 'loading'
 */
-   if (test_bit(FW_STATUS_DONE, >status))
+   if (is_fw_loading_done(buf->fwst))
return;
 
list_del_init(>pending_list);
-   set_bit(FW_STATUS_ABORT, >status);
-   complete_all(>completion);
+   fw_loading_abort(buf->fwst);
 }
 
 static void fw_load_abort(struct firmware_priv *fw_priv)
@@ -475,9 +490,6 @@ static void fw_load_abort(struct firmware_priv *fw_priv)
fw_priv->buf = NULL;
 }
 
-#define is_fw_load_aborted(buf)\
-   test_bit(FW_STATUS_ABORT, &(buf)->status)
-
 static LIST_HEAD(pending_fw_head);
 
 /* reboot notifier for avoid deadlock with usermode_lock */
@@ -577,7 +589,7 @@ static ssize_t firmware_loading_show(struct device *dev,
 
mutex_lock(_lock);
if (fw_priv->buf)
-

[RFC v0 7/8] Input: ims-pcu: use firmware_stat instead of completion

2016-07-28 Thread Daniel Wagner
From: Daniel Wagner <daniel.wag...@bmw-carit.de>

Loading firmware is an operation many drivers implement in various ways
around the completion API. And most of them do it almost in the same
way. Let's reuse the firmware_stat API which is used also by the
firmware_class loader. Apart of streamlining the firmware loading states
we also document it slightly better.

Signed-off-by: Daniel Wagner <daniel.wag...@bmw-carit.de>
---
 drivers/input/misc/ims-pcu.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index 9c0ea36..cda1fbf 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -109,7 +109,7 @@ struct ims_pcu {
 
u32 fw_start_addr;
u32 fw_end_addr;
-   struct completion async_firmware_done;
+   struct firmware_stat fw_st;
 
struct ims_pcu_buttons buttons;
struct ims_pcu_gamepad *gamepad;
@@ -940,7 +940,7 @@ static void ims_pcu_process_async_firmware(const struct 
firmware *fw,
release_firmware(fw);
 
 out:
-   complete(>async_firmware_done);
+   fw_loading_done(pcu->fw_st);
 }
 
 /*
@@ -1967,7 +1967,7 @@ static int ims_pcu_init_bootloader_mode(struct ims_pcu 
*pcu)
ims_pcu_process_async_firmware);
if (error) {
/* This error is not fatal, let userspace have another chance */
-   complete(>async_firmware_done);
+   fw_loading_abort(pcu->fw_st);
}
 
return 0;
@@ -1976,7 +1976,7 @@ static int ims_pcu_init_bootloader_mode(struct ims_pcu 
*pcu)
 static void ims_pcu_destroy_bootloader_mode(struct ims_pcu *pcu)
 {
/* Make sure our initial firmware request has completed */
-   wait_for_completion(>async_firmware_done);
+   fw_loading_wait(pcu->fw_st);
 }
 
 #define IMS_PCU_APPLICATION_MODE   0
@@ -2000,7 +2000,7 @@ static int ims_pcu_probe(struct usb_interface *intf,
pcu->bootloader_mode = id->driver_info == IMS_PCU_BOOTLOADER_MODE;
mutex_init(>cmd_mutex);
init_completion(>cmd_done);
-   init_completion(>async_firmware_done);
+   firmware_stat_init(>fw_st);
 
error = ims_pcu_parse_cdc_data(intf, pcu);
if (error)
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC v0 0/8] Reuse firmware loader helpers

2016-07-28 Thread Daniel Wagner
From: Daniel Wagner <daniel.wag...@bmw-carit.de>

Hi,

While reviewing all the complete_all() users, I realized there is
recouring pattern how the completion API is used to synchronize the
stages of the firmware loading. Since firmware_class.c contains a
fairly complete implemetation for synching the loading, it worthwhile
to export it and reuse it in drivers, At the same time one
complete_all() user is gone which is a good thing for -rt (*).

The first 2 patches are bug fixes for the test script.

Patch 3 adds the new API, and the following patches update a few
drivers.

I haven't updated all the drivers because I wanted to see first if
this is going into the right direction. Since naming is a difficult, I
am more than please to pick a better name if you have one.

cheers,
daniel

(*) Under -rt waking all waiters via complete_all() is not good. If
the complete_all() call happens in IRQ context we have an unbound
latency there. Therefore the aim is to reduce the complete_all() users
and get rid of them where possible.

Daniel Wagner (8):
  selftests: firmware: do not abort test too early
  selftests: firmware: do not clutter output
  firmware: Factor out firmware load helpers
  Input: goodix: use firmware_stat instead of completion
  ath9k_htc: use firmware_stat instead of completion
  remoteproc: use firmware_stat instead of completion
  Input: ims-pcu: use firmware_stat instead of completion
  iwl4965: use firmware_stat instead of completion

 drivers/base/firmware_class.c | 112 ++
 drivers/input/misc/ims-pcu.c  |  10 +-
 drivers/input/touchscreen/goodix.c|  10 +-
 drivers/net/wireless/ath/ath9k/hif_usb.c  |  10 +-
 drivers/net/wireless/ath/ath9k/hif_usb.h  |   2 +-
 drivers/net/wireless/intel/iwlegacy/4965-mac.c|   8 +-
 drivers/net/wireless/intel/iwlegacy/common.h  |   3 +-
 drivers/remoteproc/remoteproc_core.c  |  10 +-
 drivers/soc/ti/wkup_m3_ipc.c  |   2 +-
 include/linux/firmware.h  |  71 ++
 include/linux/remoteproc.h|   6 +-
 tools/testing/selftests/firmware/fw_filesystem.sh |   6 +-
 tools/testing/selftests/firmware/fw_userhelper.sh |   2 +-
 13 files changed, 158 insertions(+), 94 deletions(-)

-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC v0 1/8] selftests: firmware: do not abort test too early

2016-07-28 Thread Daniel Wagner
From: Daniel Wagner <daniel.wag...@bmw-carit.de>

When running the test script you will get:

kselftest/firmware/fw_userhelper.sh: line 69: echo: write error: Resource 
temporarily unavailable

and stops right there. Because the script runs with the '-e' option
which will stop the script at any error.

We should stop there because we are trying to something wrong and get an
error reported back. Instead, ignore the error message and make sure we
do not stop the script by setting the last error code to true.

Signed-off-by: Daniel Wagner <daniel.wag...@bmw-carit.de>
---
 tools/testing/selftests/firmware/fw_userhelper.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/firmware/fw_userhelper.sh 
b/tools/testing/selftests/firmware/fw_userhelper.sh
index b9983f8..2d244a7 100755
--- a/tools/testing/selftests/firmware/fw_userhelper.sh
+++ b/tools/testing/selftests/firmware/fw_userhelper.sh
@@ -66,7 +66,7 @@ NAME=$(basename "$FW")
 
 # Test failure when doing nothing (timeout works).
 echo 1 >/sys/class/firmware/timeout
-echo -n "$NAME" >"$DIR"/trigger_request
+echo -n "$NAME" >"$DIR"/trigger_request 2> /dev/null || true
 if diff -q "$FW" /dev/test_firmware >/dev/null ; then
echo "$0: firmware was not expected to match" >&2
exit 1
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC v0 4/8] Input: goodix: use firmware_stat instead of completion

2016-07-28 Thread Daniel Wagner
From: Daniel Wagner <daniel.wag...@bmw-carit.de>

Loading firmware is an operation many drivers implement in various ways
around the completion API. And most of them do it almost in the same
way. Let's reuse the firmware_stat API which is used also by the
firmware_class loader. Apart of streamlining the firmware loading states
we also document it slightly better.

Signed-off-by: Daniel Wagner <daniel.wag...@bmw-carit.de>
---
 drivers/input/touchscreen/goodix.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c 
b/drivers/input/touchscreen/goodix.c
index 240b16f..67158d3 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -47,7 +47,7 @@ struct goodix_ts_data {
u16 id;
u16 version;
const char *cfg_name;
-   struct completion firmware_loading_complete;
+   struct firmware_stat fwst;
unsigned long irq_flags;
 };
 
@@ -683,7 +683,7 @@ static void goodix_config_cb(const struct firmware *cfg, 
void *ctx)
 
 err_release_cfg:
release_firmware(cfg);
-   complete_all(>firmware_loading_complete);
+   fw_loading_done(ts->fwst);
 }
 
 static int goodix_ts_probe(struct i2c_client *client,
@@ -705,7 +705,7 @@ static int goodix_ts_probe(struct i2c_client *client,
 
ts->client = client;
i2c_set_clientdata(client, ts);
-   init_completion(>firmware_loading_complete);
+   firmware_stat_init(>fwst);
 
error = goodix_get_gpio_config(ts);
if (error)
@@ -766,7 +766,7 @@ static int goodix_ts_remove(struct i2c_client *client)
struct goodix_ts_data *ts = i2c_get_clientdata(client);
 
if (ts->gpiod_int && ts->gpiod_rst)
-   wait_for_completion(>firmware_loading_complete);
+   fw_loading_wait(ts->fwst);
 
return 0;
 }
@@ -781,7 +781,7 @@ static int __maybe_unused goodix_suspend(struct device *dev)
if (!ts->gpiod_int || !ts->gpiod_rst)
return 0;
 
-   wait_for_completion(>firmware_loading_complete);
+   fw_loading_wait(ts->fwst);
 
/* Free IRQ as IRQ pin is used as output in the suspend sequence */
goodix_free_irq(ts);
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html