Remember to initialize the TimeSync ping fields to zero so that if a
timesync_get_last_event() returns an error - we display a FrameTime that is
obviously incorrect.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Add support for the new Log class/protocol. This protocol allows modules
to send their internal logging messages to the AP in order to make
module debugging easier.
The protocol is, for now, composed a single module-initiated request.
This request contains a message and associated length. The message is
integrated in the kernel log with dev_dbg(). In order to be displayed
with 'dmesg', the following command needs to be entered first:
$ echo "file log.c +p" > /sys/kernel/debug/dynamic_debug/control
The major portion of this file was initially written by Greg KH.
Signed-off-by: Joel Porquet <porquet_joel@projectara.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
spin_[un]lock_irq() routines should be used carefully as they things can
go wrong, if they are mixed with spin_lock_irqsave() or other variants.
The main problem is that spin_[un]lock_irq() routines doesn't check if
the IRQs are already disabled/enabled on the local CPU and so
spin_unlock_irq() will forcefully enable interrupts for example.
This may not work well, if some other code was relying on interrupts
being disabled.
Use spin_lock_irqsave() and spin_unlock_restore() instead.
This patch doesn't claim that it fixes the JIRA completely, but
the issue was harder to reproduce for some iterations after this, which
was quite easy to reproduce earlier on.
Tested on EVT 2.0 with lots of debug patches to kernel and greybus.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
spin_[un]lock_irq() routines should be used carefully as they things can
go wrong, if they are mixed with spin_lock_irqsave() or other variants.
The main problem is that spin_[un]lock_irq() routines doesn't check if
the IRQs are already disabled/enabled on the local CPU and so
spin_unlock_irq() will forcefully enable interrupts for example.
This may not work well, if some other code was relying on interrupts
being disabled.
Use spin_lock_irqsave() and spin_unlock_restore() instead.
This patch doesn't claim that it fixes the JIRA completely, but
the issue was harder to reproduce for some iterations after this, which
was quite easy to reproduce earlier on.
Tested on EVT 2.0 with lots of debug patches to kernel and greybus.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
spin_[un]lock_irq() routines should be used carefully as they things can
go wrong, if they are mixed with spin_lock_irqsave() or other variants.
The main problem is that spin_[un]lock_irq() routines doesn't check if
the IRQs are already disabled/enabled on the local CPU and so
spin_unlock_irq() will forcefully enable interrupts for example.
This may not work well, if some other code was relying on interrupts
being disabled.
Use spin_lock_irqsave() and spin_unlock_restore() instead.
This patch doesn't claim that it fixes the JIRA completely, but
the issue was harder to reproduce for some iterations after this, which
was quite easy to reproduce earlier on.
Tested on EVT 2.0 with lots of debug patches to kernel and greybus.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
gb_timesync_svc_teardown() is called from gb_timesync_svc_del() and issues a
command to a remote Interface to switch off its timers. The lock ordering
is TimeSync => Interface in this case. However gb_module_del() takes an
Interface lock then calls gb_interface_del() => gb_timesync_svc_del() in
this case the lock ordering is Interface => TimeSync.
This patch fixes by removing the taking of the Interface mutex in
gb_interface_timesync_do_something(). If an Interface is present in the
TimeSync linked-list - it is by definition intf->enabled.
Reported-by: Rui Miguel Silva <rui.silva@linaro.org>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
spin_[un]lock_irq() routines should be used carefully as they things can
go wrong, if they are mixed with spin_lock_irqsave() or other variants.
The main problem is that spin_[un]lock_irq() routines doesn't check if
the IRQs are already disabled/enabled on the local CPU and so
spin_unlock_irq() will forcefully enable interrupts for example.
This may not work well, if some other code was relying on interrupts
being disabled.
Use spin_lock_irqsave() and spin_unlock_restore() instead.
This patch doesn't claim that it fixes the JIRA completely, but
the issue was harder to reproduce for some iterations after this, which
was quite easy to reproduce earlier on.
Tested on EVT 2.0 with lots of debug patches to kernel and greybus.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Jeffrey Carlyle <jcarlyle@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This reverts commit 9b891f4fda8dfd6c1d8dc16479c5f6d418a9ccc7.
We discussed this over the other thread,
[PATCH 0/2] Improve watchdog's implementation a bit,
and decided that we shouldn't be trying to hide the watchdog reboot
problem by using such patches, rather we should make sure they occur
consistently so that the real problem can be fixed.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
When building greybus against a 3.18 kernel the following error is generated:
light.c:205:9: error: implicit declaration of function 'led_sysfs_is_disabled'
[-Werror=implicit-function-declaration]
led_sysfs_is_disabled was not added until 3.19 kernel cycle
Verification:
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/include/linux/leds.h?h=linux-3.18.y
(no function led_sysfs_is_disabled defined here)
Testing Done:
- Successfully built greybus for 3.18 kernel
Signed-off-by: Michael Scott <michael.scott@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
It is believed that excessive serial messages from greybus
on suspend/resume is leading to watchdog bite.
There is still discussion going on whether ratelimiting
prints would really fix anything, except it may reduce
traffic on serial console and probably bring out real issues
in the front.
So in order to meet the alpha requirement, we all decided to
get ratelimit change "as a TEMP fix" and decide later whether
we should revert back once we get proper suspend/resume implementation.
Please follow the discussion on Jira card SW-6261.
Testing Done: Build tested against ara/main branch.
Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Reviewed-by: Konstantin Buhchev <buhchev_konstantin@projectara.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Right now greybus sdio uses the greybus operation_sync to transfer data,
this will imply an extra memcpy at greybus core that we can avoid.
Also with this change we remove the need for an extra buffer to store
intermediate copy.
So, let us create the operation and do the memory operations in the sdio
driver.
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is what we are doing elsewhere:
- Send enable/create trace events after enabling/creating stuff.
- Send disable/remove trace events before disabling/removing stuff.
This wasn't followed in a same way while disabling connections. Fix it.
Compile tested.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This will make it consistent with any other character devices we have
for greybus and let us identify greybus character devices easily.
Compiled tested only.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We need to make sure we adequately cancel and quiesce any scheduled
TimeSync synchronization operations in the case of greybus.ko being yanked
out of memory i.e. when doing an EHCI runtime suspend or just a plain
rmmod. The scenario is a new TimeSync sync operation has been scheduled.
Next gb_timesync_svc_remove() runs. In this case we should terminate any
scheduled work, terminate our ktime tracking timer and state transition to
GB_TIMESYNC_STATE_INVALID to ensure no other context may schedule any new
TimeSync operations.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Analysing a backtrace associated with the current EHCI runtime suspend code
has highlighted several places where its perfectly valid to make a
transition to GB_TIMESYNC_STATE_INACTIVE when not already in the
GB_TIMESYNC_STATE_INIT state, for example failure to issue a TimeSync
enable command to the SVC can and should legitimately call
gb_timesync_teardown() - at this point the state will be
GB_TIMESYNC_STATE_WAIT_SVC and it's legitimate and desirable to transition
to the INACTIVE state in this case. This patch fixes by removing the
restrictive and incorrect restriction on the transition to INACTIVE only
being valid when state == GB_TIMESYNC_STATE_INIT.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Tested-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
There is a scenario where gb_timesync_svc_remove() can run, attain a mutex
and call cancel_delayed_work_sync(). In the meantime a worker may already
be running and trying to attain the same mutex but will never do so as the
gb_timesync_svc_remove() path is holding the mutex and waiting on the
delayed_work_sync() to complete - leading to deadlock. This patch addresses
by calling the cancel_delayed_work_sync() before locking the relevant
mutex.
Reported-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Tested-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Add license and copyright header to the firmware.c test application.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We should be checking if any of the bundles contains a CPort with its id
set to the special value of '0', which is reserved for control CPort.
Discard the bundle in that case.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
In gb_svc_input_create() fn, on failure, wrong pointer
was being passed to input_free_device(). Correct it.
svc->input gets initialized only on successful return of this fn,
so it is absolutely wrong to pass svc->input to input_free_device().
Testing Done: Tested on EVT2.0 platform.
Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Creating the data connection at probe time makes it impossible to
support multiple inserted camera modules as they would all try to
establish a data connection to the same CPort on the AP side. Create and
destroy the data connection when configuring the streams instead, as a
single module can be streaming at a time.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
Tested-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
When the module is in the configured state, an attempt to change the
configuration must first tear down the data connection to update its
parameters, as the APB1 bridge doesn't support modifying the CSI
transmitter configuration when it is already started.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
Tested-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
When the camera pipeline can't be configured due to a failure of one of
the components (failure to start the CSI transmitter for instance),
components that have already been setup for video streaming need to be
set back to a quiescient state. This is especially important to ensure
that a stream configuration failure won't keep the UniPro links in high
speed mode forever.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
Tested-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
gb_svc_del() can be called during removal of gb-es2.ko module as well,
and in that case we would like to properly shutdown all modules and
interfaces as USB is still alive.
This requires that we don't disable the svc connection, at least for tx,
as that will be used while removing modules and interfaces.
Disable only rx to begin with, as we shouldn't be handling any requests
from the SVC. Disable tx only after all the users of svc connection are
gone.
Tested on EVT 2.0 by remove gb-es2.ko module. There are still few
errors, specially while quiescing the connections (-22), but not that
many.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We are using the mutex from gb_camera_cleanup(), which can get called
even before the mutex is initialized.
Fix it by initializing the mutex early enough.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@linaro.org>
Reviewed-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is used only to check if an existing connection already uses the
cport_id or not and doesn't really need to return pointer to the
connection.
While at it, also rename it to suit its purpose.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This patch removes few blank lines across the repository at places where
two blank lines were present together or when a blank line is present at
the start or end of a routine.
Note that this doesn't remove most of them from greybus_protocols.h as
they were added on purpose.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We need to use 'do_div()' when doing 64 bit division or modulo division
since the kernel will not pull in the gcc builtins __aeabi_ldivmod and
__aeabi_uldivmod on 32 bit builds.
Reported-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
On the gb_uart_remove() path we are forgetting to do a release_minor()
leading to a minor number leak. This is a simple one-line fix.
Tested on EVT 2.0
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Earlier I have shared a patch to rate limit err messages in audio_codec
driver. However, missed to include suggestion from Mark to do similar
changes in audio bundle & topology parser as well. Doing it now.
Testing Done: Compile tested
Fixes: 4cb3d109e5fc ("audio: Ratelimit err messages")
Signed-off-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Reviewed-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
While reordering gb_deactivate sequence to avoid protocol error this was
mistakenly added even during shutdown_tx/rx. It is supposed to be done
immediately after stop_tx and only once.
Fixes: 739f25d5f490 ("audio: Reorder gb_deactivate sequence to avoid protocol error")
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 trigger callback is used to start/stop greybus tx/rx
path. This works well for almost all scenario except few
specially handled usecases by Android Audio subsystem.
In case of Music playback followed by Incoming ringtone, above
layer tries to trigger_pause from one FE dailink and start a
fresh playback via different FE dailink. Since, same BE dailink
is used for both cases, an invalid state transition is requested
i.e. from PAUSE->START. This fails & thus causes ringtone playback
failure. With built-in codec, trigger callback is not required to
initiate data xfer unlike gb-codec driver.
This state transition should be handled in Android layer, but
since it can lead to multiple side effects for various usecase
we are trying to avoid trigger callback in gbcodec driver as well.
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>
Bring the gb_svc_intf_set_power_mode() up-to-date with the current Greybus
specification. This largely involves adding more members to the structure
sent across the wire. Also change the camera code to use the new
operation properly, with default values passed for the new necessary
arguments. The correctness of these default values is confirmed via testing
and by asking Rob Johnson.
We must make sure to zero the request structure sent across the wire, lest
bite us most cruelly, and we fix by changing the Set Power Mode
Response structure to use a __u8 rather than a __le16.
Testing Done: Took a picture with a camera module, received error code
when passing deliberately incorrect values for new parameters, got proper
-EIO and Greybus result code printed when operation stopped halfway
through. Could induce error by initializing request struct with random
nonsense, and can stop it by initializing request struct with zeroes.
Associated Firmware Changes: 1669-1671 on Android N Gerrit for SW-2945
Signed-off-by: Eli Sennesh <esennesh@leaflabs.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Some line breaks weren't required as we never crossed 80 columns, remove
them.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
There is no point keeping this code in core.c, while its only used by
hd.c. Relocate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We should be using the PM hooks available within the 'struct
device_driver', instead of adding legacy suspend/resume callbacks.
Remove them.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This routine always returns 0 or 1 and a return type of 'bool' suits it
the best. Update it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
User space doesn't break anymore with new greybus modules and its time
to make bootrom a separate module.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The "busy" SVC result codes are gone from the spec. Delete them.
Testing Done: compile.
Signed-off-by: Marti Bolivar <mbolivar@leaflabs.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
pwm_is_enabled() wasn't enabled by v4.2-rc1, but it was based of
v4.2-rc1. It was actually first included in v4.3.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Add support for greybus debug data format.
Greybus debug data format id is 0x42.
Signed-off-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
Acked-by: Laurent Pinchart <laurent.pinchart@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Core disables all connections for bundles whose interface is already
gone in order to avoid unnecessary operation timeouts during driver
disconnect.
This isn't needed for offloaded connections (as the AP can not send
requests over such connections), and in fact must not be done since only
the bundle driver currently knows how to disable I/O on such connections
in a class-specific way (this may eventually be handled by core though).
Also add comment about why connection are disabled early on forced
disconnect.
Testing Done: Tested on EVT2.
Reported-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Tested-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
I posted this once before but it got rejected for fear it would
not be clear which messages were related to Greybus.
Every trace event currently defined for Greybus is recorded in a
function whose name begins with "gb_". Every trace event reported
in /sys/kernel/debug/tracing/trace includes the name of the function
in which the event was recorded.
Get rid of the "greybus: " prefix in all of the Greybus trace
events. It just takes up precious space and is not actually
helpful. Anyone actually enabling individual trace events should
know enough about what they're doing to recognize which ones
are being enabled.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The symbol PWMF_ENABLED is defined via an enum, which is not defined
at the time the preprocessor passes through "kernel_ver.h". So we
can't use it in an #if statement expression.
Use the Linux kernel version instead.
Change-Id: Id427224b1dfecfd886fcdae89c4bcf711b616439
Signed-off-by: Alex Elder <elder@linaro.org>
AP should check for Greybus SVC Protocol Operation Status to determine if the
operation was successfully completed by the SVC
Testing Done:
- Successfully getting the rail names in the pwrmon_dummy sandbox branch
Signed-off-by: David Lin <dtwlin@google.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
In the 4.7-rc1 kernel release, PWMF_ENABLED is removed and
pwm_is_enabled() is the correct way to test if a pwm device is enabled,
so provide a version of that function that will work on all older
kernels and change the pwm.c driver to use it so that it will work on
newer kernels as well.
Tested:
Tree now builds successfully against 3.14.y, 4.4.y, 4.5.y,
4.6.y, and 4.7-rc2 kernels
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
In case of audio mgmt connection failure, GB requests would fail giving
an error message within the driver and reporting error. However there is
no error handling in above HAL and it'll keep on triggering similar
request via GB codec driver. This may overflood serial console. In one
of the instance it locked CPU for >10sec and caused a watchdog bite.
Thus ratelimit those error messages.
Testing Done: compile tested
Signed-off-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Bryan O'Donoghue
Joel Porquet
Viresh Kumar
Greg Kroah-Hartman
Michael Scott
Vaibhav Hiremath
Rui Miguel Silva
Vaibhav Hiremath
Laurent Pinchart
Vaibhav Agarwal
Eli Sennesh
Marti Bolivar
Gjorgji Rosikopulos
Johan Hovold
Alex Elder
David Lin
Jeffrey Carlyle