Audio codec driver internally maintains a struct containing info about
module's data connection. Remove unused field from this struct.
Signed-off-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Reviewed-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Currently we are using dai->name to identify data connection from list
for a module. Now since we are enabling data path based on widget,
dai->name might be invalid by the time driver receives disable request
for a widget. So, use id fetched from AIF widget->sname to identify data
connection for a module.
Signed-off-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Reviewed-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Added id field to data connection struct. This is used to identify which
data connection to use while enabling interface between module & APB.
Signed-off-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Reviewed-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Currently, SPK Amp switch is used to identify when to enable data path
between module and APB. With headset, other switch controls added, it is
not possible to use this switch to control data path. Instead path
should be established based on AIF widget.
Signed-off-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Reviewed-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Earlier, module path was enabled based on module's control switch e.g.
'SPK Amp switch'. Thus widget's name was sufficient to parse and
identify the direction. Now individual modules' path will be enabled
based on AIF widget status. So, it is required to get complete widget
details, say w->type is used to identify direction (playback/capture)
and w->sname is used to identify module's DATA connection used for
communication via greybus.
Signed-off-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Reviewed-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Currently, the stream name for the DAI driver is generically set to "GB
Audio Playback" and "GB Audio Capture". This is OK since we use a single
interface on APB1 but that could change in the future. Update the DAI
driver table entries to properly reflect the interface used.
Signed-off-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Reviewed-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Now, module is enabled/disabled based on widget event only and not
during startup/shutdown callbacks. Thus, we needn't compare codec_state
in enable/disable module helper APIs. Also, these can be further
simplified based on stream direction.
Signed-off-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Reviewed-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
As per current implementation, audio data is played from each individual
SPK module connected to endo frame. This is not a valid requirement in
case of capture/headset path. So, provide a mechanism to enable
individual module path based on it's control switch state.
Signed-off-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Reviewed-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
For most of the helper functions, goto statement is widely used. It was
originally used with an intent of single exit point for the function
with some cleanup required. This is no more the case. So, simplify code
by avoiding un-necessary gotos.
Signed-off-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Reviewed-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
In case of Mihi, SPI bus is shared between APB1 and APB2
SPI ROMs, so their FW flashing must be sequential and
arche-platform driver should make sure that they are mutual
exclusive in nature.
So this patch adds certain restrictions to the user of the
arche-platform driver,
- User can no longer flash APB1 and APB2 SPI ROM in parallel
- SPI bus becomes an resource, so user must claim it by moving
respective APB device into FW_FLASHING mode and release it
by exiting FW_FLASHING mode. User can exit FW_FLASHING mode by
switching to any other modes (ACTIVE, OFF, STANDBY).
- If APB1 is in FW_FLASHING mode, APB2 can no longer enter into
FW_FLASHING mode. User will get -EBUSY.
Having said that, while APB1 is into FW_FLASHING mode,
APB2 can independently boot from its own SPI ROM.
Testing Done: Tested by simulating usecase on EVT2.
- Made sure that APB1 and APB2 FW_FLASHING mode is mutual exclusive
in nature. Confirmed that an attempt on second device return -EBUSY.
- Added simulating code, where printed state of dummy gpio for
spi-en and verified that it shows right pin status for both APBs
Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
There is possible race condition in arche platform driver for
maintaining of ->state field, when shared with timesync driver.
Assume device booted fine, all connected modules have been enumerated
correctly. As part of suspend-resume operation we have pm_notifier
callback, where we turn off SVC & APB and coldboot on resume.
In the process of resume, all modules gets enumerated again, and
timesync driver does come into picture everytime.
So when timesync driver requests arche-platform to change the state
to TIMESYNC and in the middle of sync operation, if suspend gets
triggered, then execution lands into arche-platform->pm_notifier callback,
leading to race condition in the driver, where it checks for (state != ACTIVE)
in PM_SUSPEND_PREPARE and returns, but in PM_POST_SUSPEND it just simply
coldboots SVC & APB, which would inbalance all resources (including IRQ).
So we need a add check in PM_POST_SUSPEND, to make sure that, we only
coldboot devices if they are in to off state,
i.e. if (state != OFF) then return.
Testing Done: Done regressive suspend/resume testing on EVT2 platform.
Note that, I some time hit issue.
Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Give the USB vendor-request timeout define a more descriptive name.
Also drop the since-long obsolete comments about allowing "the SVC to do
something" and "SVC messages go down our control pipe".
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Add dedicated define for ARPC CPort requests instead of using the
default timeout for USB vendor requests.
We still allow responses to take 500 ms to arrive, but note that this
adds on top of the 500ms already allowed for a requests to be
acknowledged. This should probably be tightened up at some point.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Allow user of ARPC to pass a NULL pointer for the ARPC result.
For simple requests there may only be one error code for remote errors,
or the caller may simply not care to differentiate them.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Make sure to always set the result value for ARPC instead of forcing
every caller to do it in order to avoid compiler warnings.
The ARPC result should still be ignored unless arpc_sync returns
-EREMOTEIO.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Add a _req suffix to request message structures, and a _TYPE_ infix to
request type defines.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The host-device cport_ping callback expects the AP side cport id, but
was incorrectly passed the interface cport id.
Note that no host-device driver currently implements this callback, and
that it is soon even to be removed.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Remove some no-longer-needed braces around an error path.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The 'GB_AUDIO_TYPE_GET_TX_DELAY' and 'GB_AUDIO_TYPE_GET_RX_DELAY'
are no longer a part of the Greybus Audio Device Class Protocol
so remove support for them. The message numbers are not coalesced
to prevent compatibility issues between the AP and the module.
Testing Done: Played music using a speaker module
Signed-off-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The 'AUDIO_APBRIDGEA_TYPE_GET_TX_DELAY' and
'AUDIO_APBRIDGEA_TYPE_GET_RX_DELAY' message types have been
removed from the AP <-> APBrigdeA Audio Protocol so remove
them from the code. Do not coalesce the message type numbers
to prevent compatibility issues between the AP and APBridgeA.
Testing Done: Played music using a speaker module
Signed-off-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
If we failed to synchronize the FrameTime it would be useful to know how
many of the expected strobes arrived, for example a value of 0/5 would
indicate the SVC was completely dead but a value of 4/5 would indicate one
GPIO pulse got lost i.e. the AP was too slow reacting to an interrupt and
completely missed one of the strobe events. In either case the actual
number of strobes is a useful thing to print out.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Alex Elder <elder@linaro.org>
Reviewed-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The guard for initiating a new synchronization operation should allow for
that resync to happen in every single state except for INVALID. This patch
fixes by ensuring the guard does just that. With local testing it was
possible to break a sync to a Module. This hasn't been observed in a buglog
but should be fixed anyway.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: David Lin <dtwlin@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
It's possible the AP could miss an incoming SVC timesync pulse i.e. the AP
could have interrupts switched off for long enough that one SVC GPIO strobe
ends up over-lapping another one. TimeSync should be able to deal with this
type of transitory failure by retrying a failed synchronous TimeSync resync
up to 'n' number of times. For this patch 'n' has been set to five, which
is a hand-wavy choice that 'feels' right.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Instead of using the queue_delayed_work call for delayed work on
system_wq, use the schedule_delayed_work helper to be more consistent on
the style.
Testing Done:
- Check SVC watchdog is pining after the change.
Signed-off-by: David Lin <dtwlin@google.com>
Reviewed-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
There is an issue that when an interface failed to be enabled due to
timesync failure, a previously added control device is not deleted as
part of the error clean-up. This causes a leak in the sysfs file when
the interface is disabled. This would eventually cause this particular
interface to be unable to register future control device even after
unipro_reset. See failure logs below:
[ 906.495261] greybus 1-3.3: failed to add to timesync: -19
[ 906.516497] greybus 1-3.3: failed to re-enable interface: -19
[ 907.016016] greybus 1-3.3: Interface removed
...
[ 1623.677343] ------------[ cut here ]------------
[ 1623.681116] WARNING: at kernel/arche/fs/sysfs/dir.c:530 sysfs_add_one+0x98/0xb4()
[ 1623.681128] sysfs: cannot create duplicate filename '/bus/greybus/devices/1-3.3.ctrl'
[ 1623.681252] Call trace:
[ 1623.681265] [<ffffffc000207b40>] dump_backtrace+0x0/0x268
[ 1623.681272] [<ffffffc000207db8>] show_stack+0x10/0x1c
[ 1623.681284] [<ffffffc000ccb890>] dump_stack+0x1c/0x28
[ 1623.681295] [<ffffffc00021f9dc>] warn_slowpath_common+0x74/0x9c
[ 1623.681301] [<ffffffc00021fa60>] warn_slowpath_fmt+0x5c/0x80
[ 1623.681307] [<ffffffc000366624>] sysfs_add_one+0x94/0xb4
[ 1623.681315] [<ffffffc0003670b4>] sysfs_do_create_link_sd+0x100/0x1c8
[ 1623.681320] [<ffffffc0003671a8>] sysfs_create_link+0x2c/0x38
[ 1623.681332] [<ffffffc0005d5890>] bus_add_device+0xd8/0x190
[ 1623.681338] [<ffffffc0005d39ec>] device_add+0x2b4/0x604
[ 1623.681349] [<ffffffbffc006dfc>] gb_control_add+0x10/0x40 [greybus]
[ 1623.681362] [<ffffffbffc003dac>] gb_interface_enable+0x20c/0x3b8 [greybus]
[ 1623.681373] [<ffffffbffc002a30>] gb_module_add+0x124/0x174 [greybus]
[ 1623.681385] [<ffffffbffc0082cc>] gb_svc_intf_set_power_mode+0xdd4/0xfe8 [greybus]
[ 1623.681394] [<ffffffc00023888c>] process_one_work+0x268/0x3c8
[ 1623.681400] [<ffffffc000239a64>] worker_thread+0x204/0x358
[ 1623.681410] [<ffffffc00023f43c>] kthread+0xb8/0xc4
[ 1623.681414] ---[ end trace 44489577dd9220db ]---
[ 1623.681818] greybus 1-3.3.ctrl: failed to register control device: -17
Testing Done:
- Continuous unipro_reset stress test
Signed-off-by: David Lin <dtwlin@google.com>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Jeffrey Carlyle <jcarlyle@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Fix ARPC active-list corruption due to incorrect list-interface usage.
The corruption manifested itself as ARPC timeouts due to expected
responses not being recognised whenever more than one ARPC was active.
This could be seen for example when two interfaces were being runtime
suspended in parallel:
[ 165.739896] usb 1-1.1: invalid arpc response id received: 13
[ 165.794743] greybus 1-5.5: gb_interface_refclk_set - 0
[ 166.241202] usb 1-1.1: failed to execute ARPC: -110
Fortunately the impact of this bug has so far been limited to such
timeouts and error messages due to ARPC currently only being used for
CPort reset in the connection tear-down path.
Reported-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Add missing le16_to_cpu() to an ARPC error message in the receive path,
and also use %u to print the unsigned id.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The wrong pointer was checked for allocation failures when allocating
the ARPC response buffer, something which would lead to allocation
failures going undetected.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Replace all occurrences of the term "Ara" with "GMP" in core.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Change ARA prefix to GB_CAM for greyus camera specific
formats.
Change-Id: I1a0552516e8ea727c48085c59fc49f2409a89486
Signed-off-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
Signed-off-by: Blagovest Borisov Kolenichev <kolenichev_blagovest@projectara.com>
Acked-by: Laurent Pinchart <laurent.pinchart@linaro.org>
There's an issue that the userspace is not able to control both the
enumerated control and DAPM widget when audio bundle is in the SUSPEND
state. This patch fixes the issue by adding pm_runtime_get/put() calls
for the both controls.
Testing Done:
- Use tinymix to get and put both enumerated control and DAPM widget as
the followings, and observe audio bundle is able to wake up from
suspend.
$ tinymix "GB 3 PB source" 1
$ tinymix "GB 3 PB source"
GB 3 PB source: AIF1 >AIF2
$ tinymix "GB 3 AIF1_RX MUX" 2
$ tinymix "GB 3 AIF1_RX MUX"
GB 3 AIF1_RX MUX: Stereo Left >Right
Reported-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Signed-off-by: David Lin <dtwlin@google.com>
Reviewed-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Reviewed-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Currently, AP performs unipro_reset if SVC fails to response to its
ping. While this error recovery is best suited for the end-user
experience, errors in the UniPro network could potentially go unnoticed
by the QA and fishfooders in the development phase of the project. This
patch adds an option to trigger a kernel panic so logs can be collected
for analysis.
Testing Done:
- Reproduce issue and observe kernel panic when
watchdob_control is changed to 'panic'
Signed-off-by: David Lin <dtwlin@google.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Update documentation and sample application to capture the 'status' byte
in backend version operation and new error types in backend firmware
update operation.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The backend processor may not be ready to return the version of firmware
it is running by the time AP requests for it. The greybus specification
is updated to return 1-byte 'status' to return the error type, RETRY is
one of them.
This patch implements that in greybus now.
Note that the version structure was common across interface and backend
version requests earlier, but that is changing as well.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The specification got updated with two more status values, add their
support in greybus.
As retry isn't really an error, skip printing error messages for it as
well.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
S2 Loader doesn't support runtime PM operations currently and we will
fail to suspend/resume the bundle for firmware management protocols.
Once that happens, the bundle and its connections will be pretty much
useless as we would have tried to disable/enable all connections during
such an operation and the S2 loader doesn't expect the connections to go
away during normal operation (except in the case of mode-switch).
This patch defines a new quirk GB_INTERFACE_QUIRK_NO_PM and uses a new
interface init status value (GB_INIT_S2_LOADER_INITIALIZED) which will
be advertised by S2 Loader now in the init status.
After detecting the currently running stage as S2 Loader, the kernel
wouldn't attempt suspending or resuming the bundle.
Reviewed-by: David Lin <dtwlin@google.com>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Its a special quirk just for the bootrom as it doesn't have any PM
operations implemented. As the greybus bootrom bundle driver doesn't try
to do any PM stuff, this quirk is used only to skip bundle activate
operation currently.
Rename the GB_INTERFACE_QUIRK_NO_PM quirk to
GB_INTERFACE_QUIRK_NO_BUNDLE_ACTIVATE to suit its purpose better as the
GB_INTERFACE_QUIRK_NO_PM will be used for other quirk now.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This patch implements runtime PM support for firmware management bundle.
This guarantees that the bundle will be active, while the AP or the
Module is trying to exchange any operations over any of the CPorts.
- Firmware Management CPort:
Runtime PM get/put calls are placed around the ioctl calls, which are
all implemented as blocking ioctls.
- Component Authentication CPort:
Runtime PM get/put calls are placed around the ioctl calls, which are
all implemented as blocking ioctls.
- SPI:
Uses the interface provided by spilib.c and runtime PM get/put are
called around connection usage.
- firmware-download:
This is the most tricky one. All operations on this CPort are
initiated from the Module and not from the AP. And the AP needs to do
runtime_pm_get() before any request is received over this CPort.
The module doesn't send any request over this connection, unless the
AP has requested the module over firmware management CPort to download
a firmware package over firmware download CPort.
And so the runtime PM get/put calls around the ioctls in
fw-management.c are sufficient to handle the firmware management CPort
as well.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Clean up the incoming request handler somewhat by passing a properly
aligned header and dropping the now redundant id and type parameters.
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The response-received tracepoint is currently broken. Instead of parsing
the received message header it was tracing a bunch of zeroed fields from
an uninitialised response buffer.
Fix this by moving the tracepoint after were the response buffer is
initialised.
Fixes: 7cb496e6890e ("greybus: tracing: fix message traces")
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The operation type included in the error message printed for malformed
responses has never been correct. An uninitialised buffer was used to
retrieve the type, resulting in the type always being reported as 0.
Fix this by passing a properly aligned header to the response handler,
and drop the now redundant id and result parameters.
Fixes: cb0ef0c019ab ("operation: print message type on errors")
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Modify HID greybus driver to support runtime PM framework.
The suspend and resume fucntion have been test with tutorial-hid,
e-ink-display and gpbridge-test image by sysfs resume, all HID
buttons work well on suspend/resume.
Testing Done: Compiled and verified on EVT2, DB3.5, GPB test module
with daughter board, Red module.
Signed-off-by: Philip Yang <yang_philip@projectara.com>
Reviewed-by: David Lin <dtwlin@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The download_firmware() function isn't downloading the firmware but just
finding if one is available or not. The same applies to the error
message printed by it.
Replace 'download' with 'find' to make it sound better.
Reported-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We set timeouts using gb_bootrom_set_timeout(), which hides the internal
implementation, i.e. workqueues.
While canceling timeouts, we do cancel_delayed_work_sync(), which
exposes the internal implementation and doesn't look that clean.
Create gb_bootrom_cancel_timeout() to hide the internal implementation
here as well.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
A special race is observed in the case where the phone boots with
modules attached to it.
In that case, the AP processes a later FIRMWARE_SIZE request from the
bootrom, before processing the response to the earlier AP_READY request.
And because of that, we set the timeout from gb_bootrom_probe() after
trying to cancel it from gb_bootrom_firmware_size_request().
And with that, if the firmware package isn't available for the kernel to
directly read from, then we get a timeout print message like below:
[ 23.669764] bootrom 1-3.3.1: Firmware file 'ara_00000126_00001001_fffe0001_ffe5001a_s2l.tftf' requested
[ 24.680528] bootrom 1-3.3.1: Timed out waiting for Firmware Size Request from the Module
Note that this doesn't happen if the module is inserted into the frame,
after the phone is already booted.
This behavior is the result of an earlier commit cdd1cb3f4943
("operation: add completion work queue").
This perhaps happens only at boot time because the UNBOUND wq
(connection->wq) gets a chance to run on other CPUs, while the BOUND wq
(gb_operation_completion_wq) doesn't at that stage (24 seconds since
boot).
Setting the timeout before sending the AP_READY request fixes it for
now.
Reported-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
On failing to find a firmware image, we get these today:
bootrom 1-3.3.1: Firmware request for ara_00000126_00001001_fffe0001_ffe5001a_s2l.tftf has failed : -12
bootrom 1-3.3.1: gb_bootrom_firmware_size_request: failed to download firmware (-12)
Which are more or less duplicate, as they print error for the same root
cause.
With this patch this is all we get now:
bootrom 1-3.3.1: failed to download ara_00000126_00001001_fffe0001_ffe5001a_s2l.tftf firmware (-12)
Reported-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
