Hi, Thank you for your patch series. Full review done, I have a couple of minor review remarks inline below. Note these are really all suggestions and not must fix for this to be merged items. On 12/3/20 10:26 PM, Maximilian Luz wrote: > Add Surface System Aggregator Module core and Surface Serial Hub driver, > required for the embedded controller found on Microsoft Surface devices. > > The Surface System Aggregator Module (SSAM, SAM or Surface Aggregator) > is an embedded controller (EC) found on 4th and later generation > Microsoft Surface devices, with the exception of the Surface Go series. > This EC provides various functionality, depending on the device in > question. This can include battery status and thermal reporting (5th and > later generations), but also HID keyboard (6th+) and touchpad input > (7th+) on Surface Laptop and Surface Book 3 series devices. > > This patch provides the basic necessities for communication with the SAM > EC on 5th and later generation devices. On these devices, the EC > provides an interface that acts as serial device, called the Surface > Serial Hub (SSH). 4th generation devices, on which the EC interface is > provided via an HID-over-I2C device, are not supported by this patch. > > Specifically, this patch adds a driver for the SSH device (device HID > MSHW0084 in ACPI), as well as a controller structure and associated API. > This represents the functional core of the Surface Aggregator kernel > subsystem, introduced with this patch, and will be expanded upon in > subsequent commits. > > The SSH driver acts as the main attachment point for this subsystem and > sets-up and manages the controller structure. The controller in turn > provides a basic communication interface, allowing to send requests from > host to EC and receiving the corresponding responses, as well as > managing and receiving events, sent from EC to host. It is structured > into multiple layers, with the top layer presenting the API used by > other kernel drivers and the lower layers modeled after the serial > protocol used for communication. > > Said other drivers are then responsible for providing the (Surface model > specific) functionality accessible through the EC (e.g. battery status > reporting, thermal information, ...) via said controller structure and > API, and will be added in future commits. > > Signed-off-by: Maximilian Luz <luzmaximilian@xxxxxxxxx> > --- > > Changes in v1 (from RFC): > - add copyright lines > - change SPDX identifier to GPL-2.0+ (was GPL-2.0-or-later) > - fix bug in __ssh_ptl_queue_push, potentially leading to a deadlock > in high-traffic situations > - remove unnecessary READ_ONCE on multiple occasions > - add NULL checks for ssh_packet_get, ssh_packet_put > - add NULL checks for ssh_request_get, ssh_request_put > - guard dev_pm_ops with CONFIG_PM_SLEEP check > - mark struct ssam_request pointer as const when possible > - introduce generic retry macro with defaults > - retry base controller requests on failure > > Changes in v2: > - add transmission timeout for packets > - simplify acknowledgment timeout implementation > - simplify packet ACK handling > - restructure and simplify transmitter thread function > - restructure SSH message building > - switch to explicit RCU list for event notifiers > - use sysfs_emit for sysfs attributes > - fix memory leak on event item submission failure > - fix unaligned length access in parser > - use printk specifier for hex prefix instead of hard-coding it > - use numeric limit macros instead of casting around '-1' > - extract and document magic values > - try to make return values more consistent > - spell check comments and strings, fix typos > - unify comment style > - run checkpatch --strict, fix these and other style issues > - improve comments/documentation > > --- > MAINTAINERS | 8 + > drivers/platform/surface/Kconfig | 2 + > drivers/platform/surface/Makefile | 1 + > drivers/platform/surface/aggregator/Kconfig | 43 + > drivers/platform/surface/aggregator/Makefile | 10 + > .../platform/surface/aggregator/controller.c | 2468 +++++++++++++++++ > .../platform/surface/aggregator/controller.h | 276 ++ > drivers/platform/surface/aggregator/core.c | 781 ++++++ > .../platform/surface/aggregator/ssh_msgb.h | 205 ++ > .../surface/aggregator/ssh_packet_layer.c | 1699 ++++++++++++ > .../surface/aggregator/ssh_packet_layer.h | 187 ++ > .../platform/surface/aggregator/ssh_parser.c | 228 ++ > .../platform/surface/aggregator/ssh_parser.h | 154 + > .../surface/aggregator/ssh_request_layer.c | 1203 ++++++++ > .../surface/aggregator/ssh_request_layer.h | 143 + > include/linux/surface_aggregator/controller.h | 824 ++++++ > include/linux/surface_aggregator/serial_hub.h | 672 +++++ > 17 files changed, 8904 insertions(+) > create mode 100644 drivers/platform/surface/aggregator/Kconfig > create mode 100644 drivers/platform/surface/aggregator/Makefile > create mode 100644 drivers/platform/surface/aggregator/controller.c > create mode 100644 drivers/platform/surface/aggregator/controller.h > create mode 100644 drivers/platform/surface/aggregator/core.c > create mode 100644 drivers/platform/surface/aggregator/ssh_msgb.h > create mode 100644 drivers/platform/surface/aggregator/ssh_packet_layer.c > create mode 100644 drivers/platform/surface/aggregator/ssh_packet_layer.h > create mode 100644 drivers/platform/surface/aggregator/ssh_parser.c > create mode 100644 drivers/platform/surface/aggregator/ssh_parser.h > create mode 100644 drivers/platform/surface/aggregator/ssh_request_layer.c > create mode 100644 drivers/platform/surface/aggregator/ssh_request_layer.h > create mode 100644 include/linux/surface_aggregator/controller.h > create mode 100644 include/linux/surface_aggregator/serial_hub.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index bb4604e64b4a..64730461f4df 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11692,6 +11692,14 @@ L: platform-driver-x86@xxxxxxxxxxxxxxx > S: Supported > F: drivers/platform/surface/surfacepro3_button.c > > +MICROSOFT SURFACE SYSTEM AGGREGATOR SUBSYSTEM > +M: Maximilian Luz <luzmaximilian@xxxxxxxxx> > +S: Maintained > +W: https://github.com/linux-surface/surface-aggregator-module > +C: irc://chat.freenode.net/##linux-surface > +F: drivers/platform/surface/aggregator/ > +F: include/linux/surface_aggregator/ > + > MICROTEK X6 SCANNER > M: Oliver Neukum <oliver@xxxxxxxxxx> > S: Maintained > diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig > index 33040b0b3b79..5af22f5141ff 100644 > --- a/drivers/platform/surface/Kconfig > +++ b/drivers/platform/surface/Kconfig > @@ -56,4 +56,6 @@ config SURFACE_PRO3_BUTTON > help > This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3/4 tablet. > > +source "drivers/platform/surface/aggregator/Kconfig" > + > endif # SURFACE_PLATFORMS > diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile > index cedfb027ded1..034134fe0264 100644 > --- a/drivers/platform/surface/Makefile > +++ b/drivers/platform/surface/Makefile > @@ -7,5 +7,6 @@ > obj-$(CONFIG_SURFACE3_WMI) += surface3-wmi.o > obj-$(CONFIG_SURFACE_3_BUTTON) += surface3_button.o > obj-$(CONFIG_SURFACE_3_POWER_OPREGION) += surface3_power.o > +obj-$(CONFIG_SURFACE_AGGREGATOR) += aggregator/ > obj-$(CONFIG_SURFACE_GPE) += surface_gpe.o > obj-$(CONFIG_SURFACE_PRO3_BUTTON) += surfacepro3_button.o > diff --git a/drivers/platform/surface/aggregator/Kconfig b/drivers/platform/surface/aggregator/Kconfig > new file mode 100644 > index 000000000000..ce34941ef91b > --- /dev/null > +++ b/drivers/platform/surface/aggregator/Kconfig > @@ -0,0 +1,43 @@ > +# SPDX-License-Identifier: GPL-2.0+ > +# Copyright (C) 2019-2020 Maximilian Luz <luzmaximilian@xxxxxxxxx> > + > +menuconfig SURFACE_AGGREGATOR > + tristate "Microsoft Surface System Aggregator Module Subsystem and Drivers" > + depends on SERIAL_DEV_BUS > + depends on ACPI > + select CRC_CCITT > + help > + The Surface System Aggregator Module (Surface SAM or SSAM) is an > + embedded controller (EC) found on 5th- and later-generation Microsoft > + Surface devices (i.e. Surface Pro 5, Surface Book 2, Surface Laptop, > + and newer, with exception of Surface Go series devices). > + > + Depending on the device in question, this EC provides varying > + functionality, including: > + - EC access from ACPI via Surface ACPI Notify (5th- and 6th-generation) > + - battery status information (all devices) > + - thermal sensor access (all devices) > + - performance mode / cooling mode control (all devices) > + - clipboard detachment system control (Surface Book 2 and 3) > + - HID / keyboard input (Surface Laptops, Surface Book 3) > + > + This option controls whether the Surface SAM subsystem core will be > + built. This includes a driver for the Surface Serial Hub (SSH), which > + is the device responsible for the communication with the EC, and a > + basic kernel interface exposing the EC functionality to other client > + drivers, i.e. allowing them to make requests to the EC and receive > + events from it. Selecting this option alone will not provide any > + client drivers and therefore no functionality beyond the in-kernel > + interface. Said functionality is the responsibility of the respective > + client drivers. > + > + Note: While 4th-generation Surface devices also make use of a SAM EC, > + due to a difference in the communication interface of the controller, > + only 5th and later generations are currently supported. Specifically, > + devices using SAM-over-SSH are supported, whereas devices using > + SAM-over-HID, which is used on the 4th generation, are currently not > + supported. > + > + Choose m if you want to build the SAM subsystem core and SSH driver as > + module, y if you want to build it into the kernel and n if you don't > + want it at all. > diff --git a/drivers/platform/surface/aggregator/Makefile b/drivers/platform/surface/aggregator/Makefile > new file mode 100644 > index 000000000000..faad18d4a7f2 > --- /dev/null > +++ b/drivers/platform/surface/aggregator/Makefile > @@ -0,0 +1,10 @@ > +# SPDX-License-Identifier: GPL-2.0+ > +# Copyright (C) 2019-2020 Maximilian Luz <luzmaximilian@xxxxxxxxx> > + > +obj-$(CONFIG_SURFACE_AGGREGATOR) += surface_aggregator.o > + > +surface_aggregator-objs := core.o > +surface_aggregator-objs += ssh_parser.o > +surface_aggregator-objs += ssh_packet_layer.o > +surface_aggregator-objs += ssh_request_layer.o > +surface_aggregator-objs += controller.o > diff --git a/drivers/platform/surface/aggregator/controller.c b/drivers/platform/surface/aggregator/controller.c > new file mode 100644 > index 000000000000..8d9811cc2943 > --- /dev/null > +++ b/drivers/platform/surface/aggregator/controller.c > @@ -0,0 +1,2468 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Main SSAM/SSH controller structure and functionality. > + * > + * Copyright (C) 2019-2020 Maximilian Luz <luzmaximilian@xxxxxxxxx> > + */ > + > +#include <linux/acpi.h> > +#include <linux/atomic.h> > +#include <linux/completion.h> > +#include <linux/gpio/consumer.h> > +#include <linux/interrupt.h> > +#include <linux/kref.h> > +#include <linux/limits.h> > +#include <linux/list.h> > +#include <linux/mutex.h> > +#include <linux/rculist.h> > +#include <linux/rbtree.h> > +#include <linux/rwsem.h> > +#include <linux/serdev.h> > +#include <linux/slab.h> > +#include <linux/spinlock.h> > +#include <linux/srcu.h> > +#include <linux/types.h> > +#include <linux/workqueue.h> > + > +#include <linux/surface_aggregator/controller.h> > +#include <linux/surface_aggregator/serial_hub.h> > + > +#include "controller.h" > +#include "ssh_msgb.h" > +#include "ssh_request_layer.h" > + > + > +/* -- Safe counters. -------------------------------------------------------- */ > + > +/** > + * ssh_seq_reset() - Reset/initialize sequence ID counter. > + * @c: The counter to reset. > + */ > +static void ssh_seq_reset(struct ssh_seq_counter *c) > +{ > + WRITE_ONCE(c->value, 0); > +} > + > +/** > + * ssh_seq_next() - Get next sequence ID. > + * @c: The counter providing the sequence IDs. > + * > + * Return: Returns the next sequence ID of the counter. > + */ > +static u8 ssh_seq_next(struct ssh_seq_counter *c) > +{ > + u8 old = READ_ONCE(c->value); > + u8 new = old + 1; > + u8 ret; > + > + while (unlikely((ret = cmpxchg(&c->value, old, new)) != old)) { > + old = ret; > + new = old + 1; > + } > + > + return old; > +} > + > +/** > + * ssh_rqid_reset() - Reset/initialize request ID counter. > + * @c: The counter to reset. > + */ > +static void ssh_rqid_reset(struct ssh_rqid_counter *c) > +{ > + WRITE_ONCE(c->value, 0); > +} > + > +/** > + * ssh_rqid_next() - Get next request ID. > + * @c: The counter providing the request IDs. > + * > + * Return: Returns the next request ID of the counter, skipping any reserved > + * request IDs. > + */ > +static u16 ssh_rqid_next(struct ssh_rqid_counter *c) > +{ > + u16 old = READ_ONCE(c->value); > + u16 new = ssh_rqid_next_valid(old); > + u16 ret; > + > + while (unlikely((ret = cmpxchg(&c->value, old, new)) != old)) { > + old = ret; > + new = ssh_rqid_next_valid(old); > + } > + > + return old; > +} > + > + > +/* -- Event notifier/callbacks. --------------------------------------------- */ > +/* > + * The notifier system is based on linux/notifier.h, specifically the SRCU > + * implementation. The difference to that is, that some bits of the notifier > + * call return value can be tracked across multiple calls. This is done so > + * that handling of events can be tracked and a warning can be issued in case > + * an event goes unhandled. The idea of that warning is that it should help > + * discover and identify new/currently unimplemented features. > + */ > + > +/** > + * ssam_event_matches_notifier() - Test if an event matches a notifier. > + * @n: The event notifier to test against. > + * @event: The event to test. > + * > + * Return: Returns %true iff the given event matches the given notifier s/iff/if/ > + * according to the rules set in the notifier's event mask, %false otherwise. > + */ > +static bool ssam_event_matches_notifier(const struct ssam_event_notifier *n, > + const struct ssam_event *event) > +{ > + bool match = n->event.id.target_category == event->target_category; > + > + if (n->event.mask & SSAM_EVENT_MASK_TARGET) > + match &= n->event.reg.target_id == event->target_id; > + > + if (n->event.mask & SSAM_EVENT_MASK_INSTANCE) > + match &= n->event.id.instance == event->instance_id; > + > + return match; > +} > + > +/** > + * ssam_nfblk_call_chain() - Call event notifier callbacks of the given chain. > + * @nh: The notifier head for which the notifier callbacks should be called. > + * @event: The event data provided to the callbacks. > + * > + * Call all registered notifier callbacks in order of their priority until > + * either no notifier is left or a notifier returns a value with the > + * %SSAM_NOTIF_STOP bit set. Note that this bit is automatically set via > + * ssam_notifier_from_errno() on any non-zero error value. > + * > + * Return: Returns the notifier status value, which contains the notifier > + * status bits (%SSAM_NOTIF_HANDLED and %SSAM_NOTIF_STOP) as well as a > + * potential error value returned from the last executed notifier callback. > + * Use ssam_notifier_to_errno() to convert this value to the original error > + * value. > + */ > +static int ssam_nfblk_call_chain(struct ssam_nf_head *nh, struct ssam_event *event) > +{ > + struct ssam_event_notifier *nf; > + int ret = 0, idx; > + > + idx = srcu_read_lock(&nh->srcu); > + > + list_for_each_entry_rcu(nf, &nh->head, base.node) { > + if (ssam_event_matches_notifier(nf, event)) { > + ret = (ret & SSAM_NOTIF_STATE_MASK) | nf->base.fn(nf, event); > + if (ret & SSAM_NOTIF_STOP) > + break; > + } > + } > + > + srcu_read_unlock(&nh->srcu, idx); > + return ret; > +} > + > +/** > + * ssam_nfblk_insert() - Insert a new notifier block into the given notifier > + * list. > + * @nh: The notifier head into which the block should be inserted. > + * @nb: The notifier block to add. > + * > + * Note: This function must be synchronized by the caller with respect to other > + * insert, find, and/or remove calls. > + * > + * Return: Returns zero on success, %-EEXIST if the notifier block has already > + * been registered. > + */ > +static int ssam_nfblk_insert(struct ssam_nf_head *nh, struct ssam_notifier_block *nb) > +{ > + struct ssam_notifier_block *p; > + struct list_head *h; > + > + /* Runs under lock, no need for RCU variant. */ > + list_for_each(h, &nh->head) { > + p = list_entry(h, struct ssam_notifier_block, node); > + > + if (unlikely(p == nb)) { > + WARN(1, "double register detected"); > + return -EEXIST; > + } > + > + if (nb->priority > p->priority) > + break; > + } > + > + list_add_tail_rcu(&nb->node, h); > + return 0; > +} > + > +/** > + * ssam_nfblk_find() - Check if a notifier block is registered on the given > + * notifier head. > + * list. > + * @nh: The notifier head on which to search. > + * @nb: The notifier block to search for. > + * > + * Note: This function must be synchronized by the caller with respect to other > + * insert, find, and/or remove calls. > + * > + * Return: Returns true if the given notifier block is registered on the given > + * notifier head, false otherwise. > + */ > +static bool ssam_nfblk_find(struct ssam_nf_head *nh, struct ssam_notifier_block *nb) > +{ > + struct ssam_notifier_block *p; > + > + /* Runs under lock, no need for RCU variant. */ > + list_for_each_entry(p, &nh->head, node) { > + if (p == nb) > + return true; > + } > + > + return false; > +} > + > +/** > + * ssam_nfblk_remove() - Remove a notifier block from its notifier list. > + * @nb: The notifier block to be removed. > + * > + * Note: This function must be synchronized by the caller with respect to > + * other insert, find and/or remove calls. The caller _must_ ensure SRCU > + * synchronization by calling synchronize_srcu() with ``nh->srcu`` after > + * leaving the critical section, to ensure that the removed notifier block is > + * not in use any more. > + */ > +static void ssam_nfblk_remove(struct ssam_notifier_block *nb) > +{ > + list_del_rcu(&nb->node); > +} > + > +/** > + * ssam_nf_head_init() - Initialize the given notifier head. > + * @nh: The notifier head to initialize. > + */ > +static int ssam_nf_head_init(struct ssam_nf_head *nh) > +{ > + int status; > + > + status = init_srcu_struct(&nh->srcu); > + if (status) > + return status; > + > + INIT_LIST_HEAD(&nh->head); > + return 0; > +} > + > +/** > + * ssam_nf_head_destroy() - Deinitialize the given notifier head. > + * @nh: The notifier head to deinitialize. > + */ > +static void ssam_nf_head_destroy(struct ssam_nf_head *nh) > +{ > + cleanup_srcu_struct(&nh->srcu); > +} > + > + > +/* -- Event/notification registry. ------------------------------------------ */ > + > +/** > + * struct ssam_nf_refcount_key - Key used for event activation reference > + * counting. > + * @reg: The registry via which the event is enabled/disabled. > + * @id: The ID uniquely describing the event. > + */ > +struct ssam_nf_refcount_key { > + struct ssam_event_registry reg; > + struct ssam_event_id id; > +}; > + > +/** > + * struct ssam_nf_refcount_entry - RB-tree entry for reference counting event > + * activations. > + * @node: The node of this entry in the rb-tree. > + * @key: The key of the event. > + * @refcount: The reference-count of the event. > + * @flags: The flags used when enabling the event. > + */ > +struct ssam_nf_refcount_entry { > + struct rb_node node; > + struct ssam_nf_refcount_key key; > + int refcount; > + u8 flags; > +}; > + > +/** > + * ssam_nf_refcount_inc() - Increment reference-/activation-count of the given > + * event. > + * @nf: The notifier system reference. > + * @reg: The registry used to enable/disable the event. > + * @id: The event ID. > + * > + * Increments the reference-/activation-count associated with the specified > + * event type/ID, allocating a new entry for this event ID if necessary. A > + * newly allocated entry will have a refcount of one. > + * > + * Note: Must be synchronized by the caller with regards to other > + * ssam_nf_refcount_inc() and ssam_nf_refcount_dec() calls, e.g. via > + * ``nf->lock``. Note that this lock should also be used to ensure the > + * corresponding EC requests are sent, if necessary. You write "e.g. via ``nf->lock``", are any other locking strategies used in the other patches ? If not then it might be good to change this to just stating that nf->lock must be held and adding an assert for that. I would prefer a clear set of locking rules like this over rules like: "must be synchronized ...". Note I am naively assuming here that it is possible to come up with such a clear set of locking rules, I'm not sure if that is the case. Note in case it is possible to always take nf->lock but you are not doing so in some places because of performance concerns then I would prefer to move to a model where the caller simply always takes nf->lock. AFAICT non of the events/data going through the SSH are high frequency (iow none of them occur at a high rate of see 50 times / second or more) so I don't think that we need to focus too much on optimizing things for speed. > + * > + * Return: Returns the refcount entry on success. Returns an error pointer > + * with %-ENOSPC if there have already been %INT_MAX events of the specified > + * ID and type registered, or %-ENOMEM if the entry could not be allocated. > + */ > +static struct ssam_nf_refcount_entry > +*ssam_nf_refcount_inc(struct ssam_nf *nf, struct ssam_event_registry reg, > + struct ssam_event_id id) > +{ > + struct ssam_nf_refcount_entry *entry; > + struct ssam_nf_refcount_key key; > + struct rb_node **link = &nf->refcount.rb_node; > + struct rb_node *parent = NULL; > + int cmp; > + > + key.reg = reg; > + key.id = id; > + > + while (*link) { > + entry = rb_entry(*link, struct ssam_nf_refcount_entry, node); > + parent = *link; > + > + cmp = memcmp(&key, &entry->key, sizeof(key)); > + if (cmp < 0) { > + link = &(*link)->rb_left; > + } else if (cmp > 0) { > + link = &(*link)->rb_right; > + } else if (entry->refcount < INT_MAX) { > + entry->refcount++; > + return entry; > + } else { So we hit this only if entry->refcount == INT_MAX, right? That seems like something which should never happen, right? So maybe add a: WARN_ON(1); here? > + return ERR_PTR(-ENOSPC); > + } > + } > + > + entry = kzalloc(sizeof(*entry), GFP_KERNEL); > + if (!entry) > + return ERR_PTR(-ENOMEM); > + > + entry->key = key; > + entry->refcount = 1; > + > + rb_link_node(&entry->node, parent, link); > + rb_insert_color(&entry->node, &nf->refcount); > + > + return entry; > +} > + > +/** > + * ssam_nf_refcount_dec() - Decrement reference-/activation-count of the given > + * event. > + * @nf: The notifier system reference. > + * @reg: The registry used to enable/disable the event. > + * @id: The event ID. > + * > + * Decrements the reference-/activation-count of the specified event, > + * returning its entry. If the returned entry has a refcount of zero, the > + * caller is responsible for freeing it using kfree(). > + * > + * Note: Must be synchronized by the caller with regards to other > + * ssam_nf_refcount_inc() and ssam_nf_refcount_dec() calls, e.g. via > + * ``nf->lock``. Note that this lock should also be used to ensure the > + * corresponding EC requests are sent, if necessary. > + * > + * Return: Returns the refcount entry on success or %NULL if the entry has not > + * been found. > + */ > +static struct ssam_nf_refcount_entry > +*ssam_nf_refcount_dec(struct ssam_nf *nf, struct ssam_event_registry reg, > + struct ssam_event_id id) > +{ > + struct ssam_nf_refcount_entry *entry; > + struct ssam_nf_refcount_key key; > + struct rb_node *node = nf->refcount.rb_node; > + int cmp; > + > + key.reg = reg; > + key.id = id; > + > + while (node) { > + entry = rb_entry(node, struct ssam_nf_refcount_entry, node); > + > + cmp = memcmp(&key, &entry->key, sizeof(key)); > + if (cmp < 0) { > + node = node->rb_left; > + } else if (cmp > 0) { > + node = node->rb_right; > + } else { > + entry->refcount--; > + if (entry->refcount == 0) > + rb_erase(&entry->node, &nf->refcount); > + > + return entry; > + } > + } > + > + return NULL; > +} > + > +/** > + * ssam_nf_refcount_empty() - Test if the notification system has any > + * enabled/active events. > + * @nf: The notification system. > + */ > +static bool ssam_nf_refcount_empty(struct ssam_nf *nf) > +{ > + return RB_EMPTY_ROOT(&nf->refcount); > +} > + > +/** > + * ssam_nf_call() - Call notification callbacks for the provided event. > + * @nf: The notifier system > + * @dev: The associated device, only used for logging. > + * @rqid: The request ID of the event. > + * @event: The event provided to the callbacks. > + * > + * Execute registered callbacks in order of their priority until either no > + * callback is left or a callback returns a value with the %SSAM_NOTIF_STOP > + * bit set. Note that this bit is set automatically when converting non-zero > + * error values via ssam_notifier_from_errno() to notifier values. > + * > + * Also note that any callback that could handle an event should return a value > + * with bit %SSAM_NOTIF_HANDLED set, indicating that the event does not go > + * unhandled/ignored. In case no registered callback could handle an event, > + * this function will emit a warning. > + * > + * In case a callback failed, this function will emit an error message. > + */ > +static void ssam_nf_call(struct ssam_nf *nf, struct device *dev, u16 rqid, > + struct ssam_event *event) > +{ > + struct ssam_nf_head *nf_head; > + int status, nf_ret; > + > + if (!ssh_rqid_is_event(rqid)) { > + dev_warn(dev, "event: unsupported rqid: %#06x\n", rqid); > + return; > + } > + > + nf_head = &nf->head[ssh_rqid_to_event(rqid)]; > + nf_ret = ssam_nfblk_call_chain(nf_head, event); > + status = ssam_notifier_to_errno(nf_ret); > + > + if (status < 0) { > + dev_err(dev, > + "event: error handling event: %d (tc: %#04x, tid: %#04x, cid: %#04x, iid: %#04x)\n", > + status, event->target_category, event->target_id, > + event->command_id, event->instance_id); > + } > + > + if (!(nf_ret & SSAM_NOTIF_HANDLED)) { Maybe use "else if" here so that on an error we don't emit both the error and the unhandled event warning ? > + dev_warn(dev, > + "event: unhandled event (rqid: %#04x, tc: %#04x, tid: %#04x, cid: %#04x, iid: %#04x)\n", > + rqid, event->target_category, event->target_id, > + event->command_id, event->instance_id); > + } > +} > + > +/** > + * ssam_nf_init() - Initialize the notifier system. > + * @nf: The notifier system to initialize. > + */ > +static int ssam_nf_init(struct ssam_nf *nf) > +{ > + int i, status; > + > + for (i = 0; i < SSH_NUM_EVENTS; i++) { > + status = ssam_nf_head_init(&nf->head[i]); > + if (status) > + break; > + } > + > + if (status) { > + while (i--) > + ssam_nf_head_destroy(&nf->head[i]); > + > + return status; > + } > + > + mutex_init(&nf->lock); > + return 0; > +} > + > +/** > + * ssam_nf_destroy() - Deinitialize the notifier system. > + * @nf: The notifier system to deinitialize. > + */ > +static void ssam_nf_destroy(struct ssam_nf *nf) > +{ > + int i; > + > + for (i = 0; i < SSH_NUM_EVENTS; i++) > + ssam_nf_head_destroy(&nf->head[i]); > + > + mutex_destroy(&nf->lock); > +} > + > + > +/* -- Event/async request completion system. -------------------------------- */ > + > +#define SSAM_CPLT_WQ_NAME "ssam_cpltq" > + > +/* > + * SSAM_CPLT_WQ_BATCH - Maximum number of event item completions executed per > + * work execution. Used to prevent livelocking of the workqueue. Value chosen > + * via educated guess, may be adjusted. > + */ > +#define SSAM_CPLT_WQ_BATCH 10 > + > +/** > + * ssam_event_item_alloc() - Allocate an event item with the given payload size. > + * @len: The event payload length. > + * @flags: The flags used for allocation. > + * > + * Allocate an event item with the given payload size. Sets the item > + * operations and payload length values. The item free callback (``ops.free``) > + * should not be overwritten after this call. > + * > + * Return: Returns the newly allocated event item. > + */ > +static struct ssam_event_item *ssam_event_item_alloc(size_t len, gfp_t flags) > +{ > + struct ssam_event_item *item; > + > + item = kzalloc(struct_size(item, event.data, len), flags); > + if (!item) > + return NULL; > + > + item->event.length = len; > + return item; > +} > + > +/** > + * ssam_event_queue_push() - Push an event item to the event queue. > + * @q: The event queue. > + * @item: The item to add. > + */ > +static void ssam_event_queue_push(struct ssam_event_queue *q, > + struct ssam_event_item *item) > +{ > + spin_lock(&q->lock); > + list_add_tail(&item->node, &q->head); > + spin_unlock(&q->lock); > +} > + > +/** > + * ssam_event_queue_pop() - Pop the next event item from the event queue. > + * @q: The event queue. > + * > + * Returns and removes the next event item from the queue. Returns %NULL If > + * there is no event item left. > + */ > +static struct ssam_event_item *ssam_event_queue_pop(struct ssam_event_queue *q) > +{ > + struct ssam_event_item *item; > + > + spin_lock(&q->lock); > + item = list_first_entry_or_null(&q->head, struct ssam_event_item, node); > + if (item) > + list_del(&item->node); > + spin_unlock(&q->lock); > + > + return item; > +} > + > +/** > + * ssam_event_queue_is_empty() - Check if the event queue is empty. > + * @q: The event queue. > + */ > +static bool ssam_event_queue_is_empty(struct ssam_event_queue *q) > +{ > + bool empty; > + > + spin_lock(&q->lock); > + empty = list_empty(&q->head); > + spin_unlock(&q->lock); > + > + return empty; > +} > + > +/** > + * ssam_cplt_get_event_queue() - Get the event queue for the given parameters. > + * @cplt: The completion system on which to look for the queue. > + * @tid: The target ID of the queue. > + * @rqid: The request ID representing the event ID for which to get the queue. > + * > + * Return: Returns the event queue corresponding to the event type described > + * by the given parameters. If the request ID does not represent an event, > + * this function returns %NULL. If the target ID is not supported, this > + * function will fall back to the default target ID (``tid = 1``). > + */ > +static > +struct ssam_event_queue *ssam_cplt_get_event_queue(struct ssam_cplt *cplt, > + u8 tid, u16 rqid) > +{ > + u16 event = ssh_rqid_to_event(rqid); > + u16 tidx = ssh_tid_to_index(tid); > + > + if (!ssh_rqid_is_event(rqid)) { > + dev_err(cplt->dev, "event: unsupported request ID: %#06x\n", rqid); > + return NULL; > + } > + > + if (!ssh_tid_is_valid(tid)) { > + dev_warn(cplt->dev, "event: unsupported target ID: %u\n", tid); > + tidx = 0; > + } > + > + return &cplt->event.target[tidx].queue[event]; > +} > + > +/** > + * ssam_cplt_submit() - Submit a work item to the completion system workqueue. > + * @cplt: The completion system. > + * @work: The work item to submit. > + */ > +static bool ssam_cplt_submit(struct ssam_cplt *cplt, struct work_struct *work) > +{ > + return queue_work(cplt->wq, work); > +} > + > +/** > + * ssam_cplt_submit_event() - Submit an event to the completion system. > + * @cplt: The completion system. > + * @item: The event item to submit. > + * > + * Submits the event to the completion system by queuing it on the event item > + * queue and queuing the respective event queue work item on the completion > + * workqueue, which will eventually complete the event. > + * > + * Return: Returns zero on success, %-EINVAL if there is no event queue that > + * can handle the given event item. > + */ > +static int ssam_cplt_submit_event(struct ssam_cplt *cplt, > + struct ssam_event_item *item) > +{ > + struct ssam_event_queue *evq; > + > + evq = ssam_cplt_get_event_queue(cplt, item->event.target_id, item->rqid); > + if (!evq) > + return -EINVAL; > + > + ssam_event_queue_push(evq, item); > + ssam_cplt_submit(cplt, &evq->work); > + return 0; > +} > + > +/** > + * ssam_cplt_flush() - Flush the completion system. > + * @cplt: The completion system. > + * > + * Flush the completion system by waiting until all currently submitted work > + * items have been completed. > + * > + * Note: This function does not guarantee that all events will have been > + * handled once this call terminates. In case of a larger number of > + * to-be-completed events, the event queue work function may re-schedule its > + * work item, which this flush operation will ignore. > + * > + * This operation is only intended to, during normal operation prior to > + * shutdown, try to complete most events and requests to get them out of the > + * system while the system is still fully operational. It does not aim to > + * provide any guarantee that all of them have been handled. > + */ > +static void ssam_cplt_flush(struct ssam_cplt *cplt) > +{ > + flush_workqueue(cplt->wq); > +} > + > +static void ssam_event_queue_work_fn(struct work_struct *work) > +{ > + struct ssam_event_queue *queue; > + struct ssam_event_item *item; > + struct ssam_nf *nf; > + struct device *dev; > + unsigned int iterations = SSAM_CPLT_WQ_BATCH; > + > + queue = container_of(work, struct ssam_event_queue, work); > + nf = &queue->cplt->event.notif; > + dev = queue->cplt->dev; > + > + /* Limit number of processed events to avoid livelocking. */ > + do { > + item = ssam_event_queue_pop(queue); > + if (!item) > + return; > + > + ssam_nf_call(nf, dev, item->rqid, &item->event); > + kfree(item); > + } while (--iterations); > + > + if (!ssam_event_queue_is_empty(queue)) > + ssam_cplt_submit(queue->cplt, &queue->work); > +} > + > +/** > + * ssam_event_queue_init() - Initialize an event queue. > + * @cplt: The completion system on which the queue resides. > + * @evq: The event queue to initialize. > + */ > +static void ssam_event_queue_init(struct ssam_cplt *cplt, > + struct ssam_event_queue *evq) > +{ > + evq->cplt = cplt; > + spin_lock_init(&evq->lock); > + INIT_LIST_HEAD(&evq->head); > + INIT_WORK(&evq->work, ssam_event_queue_work_fn); > +} > + > +/** > + * ssam_cplt_init() - Initialize completion system. > + * @cplt: The completion system to initialize. > + * @dev: The device used for logging. > + */ > +static int ssam_cplt_init(struct ssam_cplt *cplt, struct device *dev) > +{ > + struct ssam_event_target *target; > + int status, c, i; > + > + cplt->dev = dev; > + > + cplt->wq = create_workqueue(SSAM_CPLT_WQ_NAME); > + if (!cplt->wq) > + return -ENOMEM; > + > + for (c = 0; c < ARRAY_SIZE(cplt->event.target); c++) { > + target = &cplt->event.target[c]; > + > + for (i = 0; i < ARRAY_SIZE(target->queue); i++) > + ssam_event_queue_init(cplt, &target->queue[i]); > + } > + > + status = ssam_nf_init(&cplt->event.notif); > + if (status) > + destroy_workqueue(cplt->wq); > + > + return status; > +} > + > +/** > + * ssam_cplt_destroy() - Deinitialize the completion system. > + * @cplt: The completion system to deinitialize. > + * > + * Deinitialize the given completion system and ensure that all pending, i.e. > + * yet-to-be-completed, event items and requests have been handled. > + */ > +static void ssam_cplt_destroy(struct ssam_cplt *cplt) > +{ > + /* > + * Note: destroy_workqueue ensures that all currently queued work will > + * be fully completed and the workqueue drained. This means that this > + * call will inherently also free any queued ssam_event_items, thus we > + * don't have to take care of that here explicitly. > + */ > + destroy_workqueue(cplt->wq); > + ssam_nf_destroy(&cplt->event.notif); > +} > + > + > +/* -- Main SSAM device structures. ------------------------------------------ */ > + > +/** > + * ssam_controller_device() - Get the &struct device associated with this > + * controller. > + * @c: The controller for which to get the device. > + * > + * Return: Returns the &struct device associated with this controller, > + * providing its lower-level transport. > + */ > +struct device *ssam_controller_device(struct ssam_controller *c) > +{ > + return ssh_rtl_get_device(&c->rtl); > +} > +EXPORT_SYMBOL_GPL(ssam_controller_device); > + > +static void __ssam_controller_release(struct kref *kref) > +{ > + struct ssam_controller *ctrl = to_ssam_controller(kref, kref); > + > + ssam_controller_destroy(ctrl); > + kfree(ctrl); > +} > + > +/** > + * ssam_controller_get() - Increment reference count of controller. > + * @c: The controller. > + * > + * Return: Returns the controller provided as input. > + */ > +struct ssam_controller *ssam_controller_get(struct ssam_controller *c) > +{ > + if (c) > + kref_get(&c->kref); > + return c; > +} > +EXPORT_SYMBOL_GPL(ssam_controller_get); > + > +/** > + * ssam_controller_put() - Decrement reference count of controller. > + * @c: The controller. > + */ > +void ssam_controller_put(struct ssam_controller *c) > +{ > + if (c) > + kref_put(&c->kref, __ssam_controller_release); > +} > +EXPORT_SYMBOL_GPL(ssam_controller_put); > + > +/** > + * ssam_controller_statelock() - Lock the controller against state transitions. > + * @c: The controller to lock. > + * > + * Lock the controller against state transitions. Holding this lock guarantees > + * that the controller will not transition between states, i.e. if the > + * controller is in state "started", when this lock has been acquired, it will > + * remain in this state at least until the lock has been released. > + * > + * Multiple clients may concurrently hold this lock. In other words: The > + * ``statelock`` functions represent the read-lock part of a r/w-semaphore. > + * Actions causing state transitions of the controller must be executed while > + * holding the write-part of this r/w-semaphore (see ssam_controller_lock() > + * and ssam_controller_unlock() for that). > + * > + * See ssam_controller_stateunlock() for the corresponding unlock function. > + */ > +void ssam_controller_statelock(struct ssam_controller *c) > +{ > + down_read(&c->lock); > +} > +EXPORT_SYMBOL_GPL(ssam_controller_statelock); > + > +/** > + * ssam_controller_stateunlock() - Unlock controller state transitions. > + * @c: The controller to unlock. > + * > + * See ssam_controller_statelock() for the corresponding lock function. > + */ > +void ssam_controller_stateunlock(struct ssam_controller *c) > +{ > + up_read(&c->lock); > +} > +EXPORT_SYMBOL_GPL(ssam_controller_stateunlock); > + > +/** > + * ssam_controller_lock() - Acquire the main controller lock. > + * @c: The controller to lock. > + * > + * This lock must be held for any state transitions, including transition to > + * suspend/resumed states and during shutdown. See ssam_controller_statelock() > + * for more details on controller locking. > + * > + * See ssam_controller_unlock() for the corresponding unlock function. > + */ > +void ssam_controller_lock(struct ssam_controller *c) > +{ > + down_write(&c->lock); > +} > + > +/* > + * ssam_controller_unlock() - Release the main controller lock. > + * @c: The controller to unlock. > + * > + * See ssam_controller_lock() for the corresponding lock function. > + */ > +void ssam_controller_unlock(struct ssam_controller *c) > +{ > + up_write(&c->lock); > +} > + > +static void ssam_handle_event(struct ssh_rtl *rtl, > + const struct ssh_command *cmd, > + const struct ssam_span *data) > +{ > + struct ssam_controller *ctrl = to_ssam_controller(rtl, rtl); > + struct ssam_event_item *item; > + > + item = ssam_event_item_alloc(data->len, GFP_KERNEL); > + if (!item) > + return; > + > + item->rqid = get_unaligned_le16(&cmd->rqid); > + item->event.target_category = cmd->tc; > + item->event.target_id = cmd->tid_in; > + item->event.command_id = cmd->cid; > + item->event.instance_id = cmd->iid; > + memcpy(&item->event.data[0], data->ptr, data->len); > + > + if (WARN_ON(ssam_cplt_submit_event(&ctrl->cplt, item))) > + kfree(item); > +} > + > +static const struct ssh_rtl_ops ssam_rtl_ops = { > + .handle_event = ssam_handle_event, > +}; > + > +static bool ssam_notifier_is_empty(struct ssam_controller *ctrl); > +static void ssam_notifier_unregister_all(struct ssam_controller *ctrl); > + > +#define SSAM_SSH_DSM_REVISION 0 > + > +/* d5e383e1-d892-4a76-89fc-f6aaae7ed5b5 */ > +static const guid_t SSAM_SSH_DSM_GUID = > + GUID_INIT(0xd5e383e1, 0xd892, 0x4a76, > + 0x89, 0xfc, 0xf6, 0xaa, 0xae, 0x7e, 0xd5, 0xb5); > + > +enum ssh_dsm_fn { > + SSH_DSM_FN_SSH_POWER_PROFILE = 0x05, > + SSH_DSM_FN_SCREEN_ON_SLEEP_IDLE_TIMEOUT = 0x06, > + SSH_DSM_FN_SCREEN_OFF_SLEEP_IDLE_TIMEOUT = 0x07, > + SSH_DSM_FN_D3_CLOSES_HANDLE = 0x08, > + SSH_DSM_FN_SSH_BUFFER_SIZE = 0x09, > +}; > + > +static int ssam_dsm_get_functions(acpi_handle handle, u64 *funcs) > +{ > + union acpi_object *obj; > + u64 mask = 0; > + int i; > + > + *funcs = 0; > + > + if (!acpi_has_method(handle, "_DSM")) > + return 0; Is this expected on some models? If yes then maybe add a comment. If not then I think this can be dropped as acpi_evaluate_dsm_typed() will then error out with -EIO which seems better then return 0 (while this is not expected). > + > + obj = acpi_evaluate_dsm_typed(handle, &SSAM_SSH_DSM_GUID, > + SSAM_SSH_DSM_REVISION, 0, NULL, > + ACPI_TYPE_BUFFER); > + if (!obj) > + return -EIO; > + > + for (i = 0; i < obj->buffer.length && i < 8; i++) > + mask |= (((u64)obj->buffer.pointer[i]) << (i * 8)); > + > + if (mask & BIT(0)) > + *funcs = mask; > + > + ACPI_FREE(obj); > + return 0; > +} > + > +static int ssam_dsm_load_u32(acpi_handle handle, u64 funcs, u64 func, u32 *ret) > +{ > + union acpi_object *obj; > + u64 val; > +> + if (!(funcs & BIT(func))) > + return 0; This exit path leaves *ret uninitialized, looking at the usage if this function I see that this is intentional, but it did stood out to me, so maybe add a comment like this ? : if (!(funcs & BIT(func))) return 0; /* Not supported leave *ret at its default value */ > + > + obj = acpi_evaluate_dsm_typed(handle, &SSAM_SSH_DSM_GUID, > + SSAM_SSH_DSM_REVISION, func, NULL, > + ACPI_TYPE_INTEGER); > + if (!obj) > + return -EIO; > + > + val = obj->integer.value; > + ACPI_FREE(obj); > + > + if (val > U32_MAX) > + return -ERANGE; > + > + *ret = val; > + return 0; > +} > + > +/** > + * ssam_controller_caps_load_from_acpi() - Load controller capabilities from > + * ACPI _DSM. > + * @handle: The handle of the ACPI controller/SSH device. > + * @caps: Where to store the capabilities in. > + * > + * Initializes the given controller capabilities with default values, then > + * checks and, if the respective _DSM functions are available, loads the > + * actual capabilities from the _DSM. > + * > + * Return: Returns zero on success, a negative error code on failure. > + */ > +static > +int ssam_controller_caps_load_from_acpi(acpi_handle handle, > + struct ssam_controller_caps *caps) > +{ > + u32 d3_closes_handle = false; > + u64 funcs; > + int status; > + > + /* Set defaults. */ > + caps->ssh_power_profile = U32_MAX; > + caps->screen_on_sleep_idle_timeout = U32_MAX; > + caps->screen_off_sleep_idle_timeout = U32_MAX; > + caps->d3_closes_handle = false; > + caps->ssh_buffer_size = U32_MAX; > + > + /* Pre-load supported DSM functions. */ > + status = ssam_dsm_get_functions(handle, &funcs); > + if (status) > + return status; > + > + /* Load actual values from ACPI, if present. */ > + status = ssam_dsm_load_u32(handle, funcs, SSH_DSM_FN_SSH_POWER_PROFILE, > + &caps->ssh_power_profile); > + if (status) > + return status; > + > + status = ssam_dsm_load_u32(handle, funcs, > + SSH_DSM_FN_SCREEN_ON_SLEEP_IDLE_TIMEOUT, > + &caps->screen_on_sleep_idle_timeout); > + if (status) > + return status; > + > + status = ssam_dsm_load_u32(handle, funcs, > + SSH_DSM_FN_SCREEN_OFF_SLEEP_IDLE_TIMEOUT, > + &caps->screen_off_sleep_idle_timeout); > + if (status) > + return status; > + > + status = ssam_dsm_load_u32(handle, funcs, SSH_DSM_FN_D3_CLOSES_HANDLE, > + &d3_closes_handle); > + if (status) > + return status; > + > + caps->d3_closes_handle = !!d3_closes_handle; > + > + status = ssam_dsm_load_u32(handle, funcs, SSH_DSM_FN_SSH_BUFFER_SIZE, > + &caps->ssh_buffer_size); > + if (status) > + return status; > + > + return 0; > +} > + > +/** > + * ssam_controller_init() - Initialize SSAM controller. > + * @ctrl: The controller to initialize. > + * @serdev: The serial device representing the underlying data transport. > + * > + * Initializes the given controller. Does neither start receiver nor > + * transmitter threads. After this call, the controller has to be hooked up to > + * the serdev core separately via &struct serdev_device_ops, relaying calls to > + * ssam_controller_receive_buf() and ssam_controller_write_wakeup(). Once the > + * controller has been hooked up, transmitter and receiver threads may be > + * started via ssam_controller_start(). These setup steps need to be completed > + * before controller can be used for requests. > + */ > +int ssam_controller_init(struct ssam_controller *ctrl, > + struct serdev_device *serdev) > +{ > + acpi_handle handle = ACPI_HANDLE(&serdev->dev); > + int status; > + > + init_rwsem(&ctrl->lock); > + kref_init(&ctrl->kref); > + > + status = ssam_controller_caps_load_from_acpi(handle, &ctrl->caps); > + if (status) > + return status; > + > + dev_dbg(&serdev->dev, > + "device capabilities:\n" > + " ssh_power_profile: %u\n" > + " ssh_buffer_size: %u\n" > + " screen_on_sleep_idle_timeout: %u\n" > + " screen_off_sleep_idle_timeout: %u\n" > + " d3_closes_handle: %u\n", > + ctrl->caps.ssh_power_profile, > + ctrl->caps.ssh_buffer_size, > + ctrl->caps.screen_on_sleep_idle_timeout, > + ctrl->caps.screen_off_sleep_idle_timeout, > + ctrl->caps.d3_closes_handle); > + > + ssh_seq_reset(&ctrl->counter.seq); > + ssh_rqid_reset(&ctrl->counter.rqid); > + > + /* Initialize event/request completion system. */ > + status = ssam_cplt_init(&ctrl->cplt, &serdev->dev); > + if (status) > + return status; > + > + /* Initialize request and packet transport layers. */ > + status = ssh_rtl_init(&ctrl->rtl, serdev, &ssam_rtl_ops); > + if (status) { > + ssam_cplt_destroy(&ctrl->cplt); > + return status; > + } > + > + /* > + * Set state via write_once even though we expect to be in an > + * exclusive context, due to smoke-testing in > + * ssam_request_sync_submit(). > + */ > + WRITE_ONCE(ctrl->state, SSAM_CONTROLLER_INITIALIZED); > + return 0; > +} > + > +/** > + * ssam_controller_start() - Start the receiver and transmitter threads of the > + * controller. > + * @ctrl: The controller. > + * > + * Note: When this function is called, the controller should be properly > + * hooked up to the serdev core via &struct serdev_device_ops. Please refer > + * to ssam_controller_init() for more details on controller initialization. > + * > + * This function must be called from an exclusive context with regards to the > + * state, if necessary, by locking the controller via ssam_controller_lock(). Again you are being a bit hand-wavy (I assume you know what I mean by that) wrt the locking requirements. If possible I would prefer clearly spelled out locking requirements in the form of "this and that lock must be held when calling this function". Preferably backed-up by lockdep_assert-s asserting these conditions. And maybe if you are a bit stricter with always holding the lock when calling this, you can also drop the WRITE_ONCE and the comment about it (in all places where you do this). Note, that like my remarks around the locking docs for ssam_nf_refcount_inc() this assumes that it is possible to come up with such a clear set of locking rules. If that for some reason is not straight forwardd, then maybe add some docs documenting the locking expectations somewhere instead ? > + */ > +int ssam_controller_start(struct ssam_controller *ctrl) > +{ > + int status; > + > + if (ctrl->state != SSAM_CONTROLLER_INITIALIZED) > + return -EINVAL; > + > + status = ssh_rtl_start(&ctrl->rtl); > + if (status) > + return status; > + > + /* > + * Set state via write_once even though we expect to be locked/in an > + * exclusive context, due to smoke-testing in > + * ssam_request_sync_submit(). > + */ > + WRITE_ONCE(ctrl->state, SSAM_CONTROLLER_STARTED); > + return 0; > +} > + > +/* > + * SSAM_CTRL_SHUTDOWN_FLUSH_TIMEOUT - Timeout for flushing requests during > + * shutdown. > + * > + * Chosen to be larger than one full request timeout, including packets timing > + * out. This value should give ample time to complete any outstanding requests > + * during normal operation and account for the odd package timeout. > + */ > +#define SSAM_CTRL_SHUTDOWN_FLUSH_TIMEOUT msecs_to_jiffies(5000) > + > +/** > + * ssam_controller_shutdown() - Shut down the controller. > + * @ctrl: The controller. > + * > + * Shuts down the controller by flushing all pending requests and stopping the > + * transmitter and receiver threads. All requests submitted after this call > + * will fail with %-ESHUTDOWN. While it is discouraged to do so, this function > + * is safe to use in parallel with ongoing request submission. > + * > + * In the course of this shutdown procedure, all currently registered > + * notifiers will be unregistered. It is, however, strongly recommended to not > + * rely on this behavior, and instead the party registering the notifier > + * should unregister it before the controller gets shut down, e.g. via the > + * SSAM bus which guarantees client devices to be removed before a shutdown. > + * > + * Note that events may still be pending after this call, but, due to the > + * notifiers being unregistered, these events will be dropped when the > + * controller is subsequently destroyed via ssam_controller_destroy(). > + * > + * This function must be called from an exclusive context with regards to the > + * state, if necessary, by locking the controller via ssam_controller_lock(). Same comment as earlier wrt the locking. > + */ > +void ssam_controller_shutdown(struct ssam_controller *ctrl) > +{ > + enum ssam_controller_state s = ctrl->state; > + int status; > + > + if (s == SSAM_CONTROLLER_UNINITIALIZED || s == SSAM_CONTROLLER_STOPPED) > + return; > + > + /* > + * Try to flush pending events and requests while everything still > + * works. Note: There may still be packets and/or requests in the > + * system after this call (e.g. via control packets submitted by the > + * packet transport layer or flush timeout / failure, ...). Those will > + * be handled with the ssh_rtl_shutdown() call below. > + */ > + status = ssh_rtl_flush(&ctrl->rtl, SSAM_CTRL_SHUTDOWN_FLUSH_TIMEOUT); > + if (status) { > + ssam_err(ctrl, "failed to flush request transport layer: %d\n", > + status); > + } > + > + /* Try to flush all currently completing requests and events. */ > + ssam_cplt_flush(&ctrl->cplt); > + > + /* > + * We expect all notifiers to have been removed by the respective client > + * driver that set them up at this point. If this warning occurs, some > + * client driver has not done that... > + */ > + WARN_ON(!ssam_notifier_is_empty(ctrl)); > + > + /* > + * Nevertheless, we should still take care of drivers that don't behave > + * well. Thus disable all enabled events, unregister all notifiers. > + */ > + ssam_notifier_unregister_all(ctrl); > + > + /* > + * Cancel remaining requests. Ensure no new ones can be queued and stop > + * threads. > + */ > + ssh_rtl_shutdown(&ctrl->rtl); > + > + /* > + * Set state via write_once even though we expect to be locked/in an > + * exclusive context, due to smoke-testing in > + * ssam_request_sync_submit(). > + */ > + WRITE_ONCE(ctrl->state, SSAM_CONTROLLER_STOPPED); > + ctrl->rtl.ptl.serdev = NULL; > +} > + > +/** > + * ssam_controller_destroy() - Destroy the controller and free its resources. > + * @ctrl: The controller. > + * > + * Ensures that all resources associated with the controller get freed. This > + * function should only be called after the controller has been stopped via > + * ssam_controller_shutdown(). In general, this function should not be called > + * directly. The only valid place to call this function directly is during > + * initialization, before the controller has been fully initialized and passed > + * to other processes. This function is called automatically when the > + * reference count of the controller reaches zero. > + * > + * Must be called from an exclusive context with regards to the controller > + * state. Same comment as earlier wrt the locking. > + */ > +void ssam_controller_destroy(struct ssam_controller *ctrl) > +{ > + if (ctrl->state == SSAM_CONTROLLER_UNINITIALIZED) > + return; > + > + WARN_ON(ctrl->state != SSAM_CONTROLLER_STOPPED); > + > + /* > + * Note: New events could still have been received after the previous > + * flush in ssam_controller_shutdown, before the request transport layer > + * has been shut down. At this point, after the shutdown, we can be sure > + * that no new events will be queued. The call to ssam_cplt_destroy will > + * ensure that those remaining are being completed and freed. > + */ > + > + /* Actually free resources. */ > + ssam_cplt_destroy(&ctrl->cplt); > + ssh_rtl_destroy(&ctrl->rtl); > + > + /* > + * Set state via write_once even though we expect to be locked/in an > + * exclusive context, due to smoke-testing in > + * ssam_request_sync_submit(). > + */ > + WRITE_ONCE(ctrl->state, SSAM_CONTROLLER_UNINITIALIZED); > +} > + > +/** > + * ssam_controller_suspend() - Suspend the controller. > + * @ctrl: The controller to suspend. > + * > + * Marks the controller as suspended. Note that display-off and D0-exit > + * notifications have to be sent manually before transitioning the controller > + * into the suspended state via this function. > + * > + * See ssam_controller_resume() for the corresponding resume function. > + * > + * Return: Returns %-EINVAL if the controller is currently not in the > + * "started" state. > + */ > +int ssam_controller_suspend(struct ssam_controller *ctrl) > +{ > + ssam_controller_lock(ctrl); > + > + if (ctrl->state != SSAM_CONTROLLER_STARTED) { > + ssam_controller_unlock(ctrl); > + return -EINVAL; > + } > + > + ssam_dbg(ctrl, "pm: suspending controller\n"); > + > + /* > + * Set state via write_once even though we're locked, due to > + * smoke-testing in ssam_request_sync_submit(). > + */ > + WRITE_ONCE(ctrl->state, SSAM_CONTROLLER_SUSPENDED); > + > + ssam_controller_unlock(ctrl); > + return 0; > +} > + > +/** > + * ssam_controller_resume() - Resume the controller from suspend. > + * @ctrl: The controller to resume. > + * > + * Resume the controller from the suspended state it was put into via > + * ssam_controller_suspend(). This function does not issue display-on and > + * D0-entry notifications. If required, those have to be sent manually after > + * this call. > + * > + * Return: Returns %-EINVAL if the controller is currently not suspended. > + */ > +int ssam_controller_resume(struct ssam_controller *ctrl) > +{ > + ssam_controller_lock(ctrl); > + > + if (ctrl->state != SSAM_CONTROLLER_SUSPENDED) { > + ssam_controller_unlock(ctrl); > + return -EINVAL; > + } > + > + ssam_dbg(ctrl, "pm: resuming controller\n"); > + > + /* > + * Set state via write_once even though we're locked, due to > + * smoke-testing in ssam_request_sync_submit(). > + */ > + WRITE_ONCE(ctrl->state, SSAM_CONTROLLER_STARTED); > + > + ssam_controller_unlock(ctrl); > + return 0; > +} > + > + > +/* -- Top-level request interface ------------------------------------------- */ > + > +/** > + * ssam_request_write_data() - Construct and write SAM request message to > + * buffer. > + * @buf: The buffer to write the data to. > + * @ctrl: The controller via which the request will be sent. > + * @spec: The request data and specification. > + * > + * Constructs a SAM/SSH request message and writes it to the provided buffer. > + * The request and transport counters, specifically RQID and SEQ, will be set > + * in this call. These counters are obtained from the controller. It is thus > + * only valid to send the resulting message via the controller specified here. > + * > + * For calculation of the required buffer size, refer to the > + * SSH_COMMAND_MESSAGE_LENGTH() macro. > + * > + * Return: Returns the number of bytes used in the buffer on success. Returns > + * %-EINVAL if the payload length provided in the request specification is too > + * large (larger than %SSH_COMMAND_MAX_PAYLOAD_SIZE) or if the provided buffer > + * is too small. > + */ > +ssize_t ssam_request_write_data(struct ssam_span *buf, > + struct ssam_controller *ctrl, > + const struct ssam_request *spec) > +{ > + struct msgbuf msgb; > + u16 rqid; > + u8 seq; > + > + if (spec->length > SSH_COMMAND_MAX_PAYLOAD_SIZE) > + return -EINVAL; > + > + if (SSH_COMMAND_MESSAGE_LENGTH(spec->length) > buf->len) > + return -EINVAL; > + > + msgb_init(&msgb, buf->ptr, buf->len); > + seq = ssh_seq_next(&ctrl->counter.seq); > + rqid = ssh_rqid_next(&ctrl->counter.rqid); > + msgb_push_cmd(&msgb, seq, rqid, spec); > + > + return msgb_bytes_used(&msgb); > +} > +EXPORT_SYMBOL_GPL(ssam_request_write_data); > + > +static void ssam_request_sync_complete(struct ssh_request *rqst, > + const struct ssh_command *cmd, > + const struct ssam_span *data, int status) > +{ > + struct ssh_rtl *rtl = ssh_request_rtl(rqst); > + struct ssam_request_sync *r; > + > + r = container_of(rqst, struct ssam_request_sync, base); > + r->status = status; > + > + if (r->resp) > + r->resp->length = 0; > + > + if (status) { > + rtl_dbg_cond(rtl, "rsp: request failed: %d\n", status); > + return; > + } > + > + if (!data) /* Handle requests without a response. */ > + return; > + > + if (!r->resp || !r->resp->pointer) { > + if (data->len) > + rtl_warn(rtl, "rsp: no response buffer provided, dropping data\n"); > + return; > + } > + > + if (data->len > r->resp->capacity) { > + rtl_err(rtl, > + "rsp: response buffer too small, capacity: %zu bytes, got: %zu bytes\n", > + r->resp->capacity, data->len); > + r->status = -ENOSPC; > + return; > + } > + > + r->resp->length = data->len; > + memcpy(r->resp->pointer, data->ptr, data->len); > +} > + > +static void ssam_request_sync_release(struct ssh_request *rqst) > +{ > + complete_all(&container_of(rqst, struct ssam_request_sync, base)->comp); > +} > + > +static const struct ssh_request_ops ssam_request_sync_ops = { > + .release = ssam_request_sync_release, > + .complete = ssam_request_sync_complete, > +}; > + > +/** > + * ssam_request_sync_alloc() - Allocate a synchronous request. > + * @payload_len: The length of the request payload. > + * @flags: Flags used for allocation. > + * @rqst: Where to store the pointer to the allocated request. > + * @buffer: Where to store the buffer descriptor for the message buffer of > + * the request. > + * > + * Allocates a synchronous request with corresponding message buffer. The > + * request still needs to be initialized ssam_request_sync_init() before > + * it can be submitted, and the message buffer data must still be set to the > + * returned buffer via ssam_request_sync_set_data() after it has been filled, > + * if need be with adjusted message length. > + * > + * After use, the request and its corresponding message buffer should be freed > + * via ssam_request_sync_free(). The buffer must not be freed separately. > + * > + * Return: Returns zero on success, %-ENOMEM if the request could not be > + * allocated. > + */ > +int ssam_request_sync_alloc(size_t payload_len, gfp_t flags, > + struct ssam_request_sync **rqst, > + struct ssam_span *buffer) > +{ > + size_t msglen = SSH_COMMAND_MESSAGE_LENGTH(payload_len); > + > + *rqst = kzalloc(sizeof(**rqst) + msglen, flags); > + if (!*rqst) > + return -ENOMEM; > + > + buffer->ptr = (u8 *)(*rqst + 1); > + buffer->len = msglen; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(ssam_request_sync_alloc); > + > +/** > + * ssam_request_sync_free() - Free a synchronous request. > + * @rqst: The request to be freed. > + * > + * Free a synchronous request and its corresponding buffer allocated with > + * ssam_request_sync_alloc(). Do not use for requests allocated on the stack > + * or via any other function. > + * > + * Warning: The caller must ensure that the request is not in use any more. > + * I.e. the caller must ensure that it has the only reference to the request > + * and the request is not currently pending. This means that the caller has > + * either never submitted the request, request submission has failed, or the > + * caller has waited until the submitted request has been completed via > + * ssam_request_sync_wait(). > + */ > +void ssam_request_sync_free(struct ssam_request_sync *rqst) > +{ > + kfree(rqst); > +} > +EXPORT_SYMBOL_GPL(ssam_request_sync_free); > + > +/** > + * ssam_request_sync_init() - Initialize a synchronous request struct. > + * @rqst: The request to initialize. > + * @flags: The request flags. > + * > + * Initializes the given request struct. Does not initialize the request > + * message data. This has to be done explicitly after this call via > + * ssam_request_sync_set_data() and the actual message data has to be written > + * via ssam_request_write_data(). > + */ > +void ssam_request_sync_init(struct ssam_request_sync *rqst, > + enum ssam_request_flags flags) > +{ > + ssh_request_init(&rqst->base, flags, &ssam_request_sync_ops); > + init_completion(&rqst->comp); > + rqst->resp = NULL; > + rqst->status = 0; > +} > +EXPORT_SYMBOL_GPL(ssam_request_sync_init); > + > +/** > + * ssam_request_sync_submit() - Submit a synchronous request. > + * @ctrl: The controller with which to submit the request. > + * @rqst: The request to submit. > + * > + * Submit a synchronous request. The request has to be initialized and > + * properly set up, including response buffer (may be %NULL if no response is > + * expected) and command message data. This function does not wait for the > + * request to be completed. > + * > + * If this function succeeds, ssam_request_sync_wait() must be used to ensure > + * that the request has been completed before the response data can be > + * accessed and/or the request can be freed. On failure, the request may > + * immediately be freed. > + * > + * This function may only be used if the controller is active, i.e. has been > + * initialized and not suspended. > + */ > +int ssam_request_sync_submit(struct ssam_controller *ctrl, > + struct ssam_request_sync *rqst) > +{ > + int status; > + > + /* > + * This is only a superficial check. In general, the caller needs to > + * ensure that the controller is initialized and is not (and does not > + * get) suspended during use, i.e. until the request has been completed > + * (if _absolutely_ necessary, by use of ssam_controller_statelock/ > + * ssam_controller_stateunlock, but something like ssam_client_link > + * should be preferred as this needs to last until the request has been > + * completed). > + * > + * Note that it is actually safe to use this function while the > + * controller is in the process of being shut down (as ssh_rtl_submit > + * is safe with regards to this), but it is generally discouraged to do > + * so. > + */ > + if (WARN_ON(READ_ONCE(ctrl->state) != SSAM_CONTROLLER_STARTED)) { > + ssh_request_put(&rqst->base); > + return -ENODEV; > + } > + > + status = ssh_rtl_submit(&ctrl->rtl, &rqst->base); > + ssh_request_put(&rqst->base); > + > + return status; > +} > +EXPORT_SYMBOL_GPL(ssam_request_sync_submit); > + > +/** > + * ssam_request_sync() - Execute a synchronous request. > + * @ctrl: The controller via which the request will be submitted. > + * @spec: The request specification and payload. > + * @rsp: The response buffer. > + * > + * Allocates a synchronous request with its message data buffer on the heap > + * via ssam_request_sync_alloc(), fully initializes it via the provided > + * request specification, submits it, and finally waits for its completion > + * before freeing it and returning its status. > + * > + * Return: Returns the status of the request or any failure during setup. > + */ > +int ssam_request_sync(struct ssam_controller *ctrl, > + const struct ssam_request *spec, > + struct ssam_response *rsp) > +{ > + struct ssam_request_sync *rqst; > + struct ssam_span buf; > + ssize_t len; > + int status; > + > + status = ssam_request_sync_alloc(spec->length, GFP_KERNEL, &rqst, &buf); > + if (status) > + return status; > + > + ssam_request_sync_init(rqst, spec->flags); > + ssam_request_sync_set_resp(rqst, rsp); > + > + len = ssam_request_write_data(&buf, ctrl, spec); > + if (len < 0) { > + ssam_request_sync_free(rqst); > + return len; > + } > + > + ssam_request_sync_set_data(rqst, buf.ptr, len); > + > + status = ssam_request_sync_submit(ctrl, rqst); > + if (!status) > + status = ssam_request_sync_wait(rqst); > + > + ssam_request_sync_free(rqst); > + return status; > +} > +EXPORT_SYMBOL_GPL(ssam_request_sync); > + > +/** > + * ssam_request_sync_with_buffer() - Execute a synchronous request with the > + * provided buffer as back-end for the message buffer. > + * @ctrl: The controller via which the request will be submitted. > + * @spec: The request specification and payload. > + * @rsp: The response buffer. > + * @buf: The buffer for the request message data. > + * > + * Allocates a synchronous request struct on the stack, fully initializes it > + * using the provided buffer as message data buffer, submits it, and then > + * waits for its completion before returning its status. The > + * SSH_COMMAND_MESSAGE_LENGTH() macro can be used to compute the required > + * message buffer size. > + * > + * This function does essentially the same as ssam_request_sync(), but instead > + * of dynamically allocating the request and message data buffer, it uses the > + * provided message data buffer and stores the (small) request struct on the > + * heap. > + * > + * Return: Returns the status of the request or any failure during setup. > + */ > +int ssam_request_sync_with_buffer(struct ssam_controller *ctrl, > + const struct ssam_request *spec, > + struct ssam_response *rsp, > + struct ssam_span *buf) > +{ > + struct ssam_request_sync rqst; > + ssize_t len; > + int status; > + > + ssam_request_sync_init(&rqst, spec->flags); > + ssam_request_sync_set_resp(&rqst, rsp); > + > + len = ssam_request_write_data(buf, ctrl, spec); > + if (len < 0) > + return len; > + > + ssam_request_sync_set_data(&rqst, buf->ptr, len); > + > + status = ssam_request_sync_submit(ctrl, &rqst); > + if (!status) > + status = ssam_request_sync_wait(&rqst); > + > + return status; > +} > +EXPORT_SYMBOL_GPL(ssam_request_sync_with_buffer); > + > + > +/* -- Internal SAM requests. ------------------------------------------------ */ > + > +static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_get_firmware_version, __le32, { > + .target_category = SSAM_SSH_TC_SAM, > + .target_id = 0x01, > + .command_id = 0x13, > + .instance_id = 0x00, > +}); > + > +static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_display_off, u8, { > + .target_category = SSAM_SSH_TC_SAM, > + .target_id = 0x01, > + .command_id = 0x15, > + .instance_id = 0x00, > +}); > + > +static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_display_on, u8, { > + .target_category = SSAM_SSH_TC_SAM, > + .target_id = 0x01, > + .command_id = 0x16, > + .instance_id = 0x00, > +}); > + > +static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_d0_exit, u8, { > + .target_category = SSAM_SSH_TC_SAM, > + .target_id = 0x01, > + .command_id = 0x33, > + .instance_id = 0x00, > +}); > + > +static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_d0_entry, u8, { > + .target_category = SSAM_SSH_TC_SAM, > + .target_id = 0x01, > + .command_id = 0x34, > + .instance_id = 0x00, > +}); > + > +/** > + * struct ssh_notification_params - Command payload to enable/disable SSH > + * notifications. > + * @target_category: The target category for which notifications should be > + * enabled/disabled. > + * @flags: Flags determining how notifications are being sent. > + * @request_id: The request ID that is used to send these notifications. > + * @instance_id: The specific instance in the given target category for > + * which notifications should be enabled. > + */ > +struct ssh_notification_params { > + u8 target_category; > + u8 flags; > + __le16 request_id; > + u8 instance_id; > +} __packed; > + > +static_assert(sizeof(struct ssh_notification_params) == 5); > + > +static int __ssam_ssh_event_request(struct ssam_controller *ctrl, > + struct ssam_event_registry reg, u8 cid, > + struct ssam_event_id id, u8 flags) > +{ > + struct ssh_notification_params params; > + struct ssam_request rqst; > + struct ssam_response result; > + int status; > + > + u16 rqid = ssh_tc_to_rqid(id.target_category); > + u8 buf = 0; > + > + /* Only allow RQIDs that lie within the event spectrum. */ > + if (!ssh_rqid_is_event(rqid)) > + return -EINVAL; > + > + params.target_category = id.target_category; > + params.instance_id = id.instance; > + params.flags = flags; > + put_unaligned_le16(rqid, ¶ms.request_id); > + > + rqst.target_category = reg.target_category; > + rqst.target_id = reg.target_id; > + rqst.command_id = cid; > + rqst.instance_id = 0x00; > + rqst.flags = SSAM_REQUEST_HAS_RESPONSE; > + rqst.length = sizeof(params); > + rqst.payload = (u8 *)¶ms; > + > + result.capacity = sizeof(buf); > + result.length = 0; > + result.pointer = &buf; > + > + status = ssam_retry(ssam_request_sync_onstack, ctrl, &rqst, &result, > + sizeof(params)); > + > + return status < 0 ? status : buf; > +} > + > +/** > + * ssam_ssh_event_enable() - Enable SSH event. > + * @ctrl: The controller for which to enable the event. > + * @reg: The event registry describing what request to use for enabling and > + * disabling the event. > + * @id: The event identifier. > + * @flags: The event flags. > + * > + * Enables the specified event on the EC. This function does not manage > + * reference counting of enabled events and is basically only a wrapper for > + * the raw EC request. If the specified event is already enabled, the EC will > + * ignore this request. > + * > + * Return: Returns the status of the executed SAM request (zero on success and > + * negative on direct failure) or %-EPROTO if the request response indicates a > + * failure. > + */ > +static int ssam_ssh_event_enable(struct ssam_controller *ctrl, > + struct ssam_event_registry reg, > + struct ssam_event_id id, u8 flags) > +{ > + int status; > + > + status = __ssam_ssh_event_request(ctrl, reg, reg.cid_enable, id, flags); > + > + if (status < 0 && status != -EINVAL) { > + ssam_err(ctrl, > + "failed to enable event source (tc: %#04x, iid: %#04x, reg: %#04x)\n", > + id.target_category, id.instance, reg.target_category); > + } > + > + if (status > 0) { > + ssam_err(ctrl, > + "unexpected result while enabling event source: %#04x (tc: %#04x, iid: %#04x, reg: %#04x)\n", > + status, id.target_category, id.instance, reg.target_category); > + return -EPROTO; > + } > + > + return status; > +} > + > +/** > + * ssam_ssh_event_disable() - Disable SSH event. > + * @ctrl: The controller for which to disable the event. > + * @reg: The event registry describing what request to use for enabling and > + * disabling the event (must be same as used when enabling the event). > + * @id: The event identifier. > + * @flags: The event flags (likely ignored for disabling of events). > + * > + * Disables the specified event on the EC. This function does not manage > + * reference counting of enabled events and is basically only a wrapper for > + * the raw EC request. If the specified event is already disabled, the EC will > + * ignore this request. > + * > + * Return: Returns the status of the executed SAM request (zero on success and > + * negative on direct failure) or %-EPROTO if the request response indicates a > + * failure. > + */ > +static int ssam_ssh_event_disable(struct ssam_controller *ctrl, > + struct ssam_event_registry reg, > + struct ssam_event_id id, u8 flags) > +{ > + int status; > + > + status = __ssam_ssh_event_request(ctrl, reg, reg.cid_enable, id, flags); > + > + if (status < 0 && status != -EINVAL) { > + ssam_err(ctrl, > + "failed to disable event source (tc: %#04x, iid: %#04x, reg: %#04x)\n", > + id.target_category, id.instance, reg.target_category); > + } > + > + if (status > 0) { > + ssam_err(ctrl, > + "unexpected result while disabling event source: %#04x (tc: %#04x, iid: %#04x, reg: %#04x)\n", > + status, id.target_category, id.instance, reg.target_category); > + return -EPROTO; > + } > + > + return status; > +} > + > + > +/* -- Wrappers for internal SAM requests. ----------------------------------- */ > + > +/** > + * ssam_get_firmware_version() - Get the SAM/EC firmware version. > + * @ctrl: The controller. > + * @version: Where to store the version number. > + * > + * Return: Returns zero on success or the status of the executed SAM request > + * if that request failed. > + */ > +int ssam_get_firmware_version(struct ssam_controller *ctrl, u32 *version) > +{ > + __le32 __version; > + int status; > + > + status = ssam_retry(ssam_ssh_get_firmware_version, ctrl, &__version); > + if (status) > + return status; > + > + *version = le32_to_cpu(__version); > + return 0; > +} > + > +/** > + * ssam_ctrl_notif_display_off() - Notify EC that the display has been turned > + * off. > + * @ctrl: The controller. > + * > + * Notify the EC that the display has been turned off and the driver may enter > + * a lower-power state. This will prevent events from being sent directly. > + * Rather, the EC signals an event by pulling the wakeup GPIO high for as long > + * as there are pending events. The events then need to be manually released, > + * one by one, via the GPIO callback request. All pending events accumulated > + * during this state can also be released by issuing the display-on > + * notification, e.g. via ssam_ctrl_notif_display_on(), which will also reset > + * the GPIO. > + * > + * On some devices, specifically ones with an integrated keyboard, the keyboard > + * backlight will be turned off by this call. > + * > + * This function will only send the display-off notification command if > + * display notifications are supported by the EC. Currently all known devices > + * support these notifications. > + * > + * Use ssam_ctrl_notif_display_on() to reverse the effects of this function. > + * > + * Return: Returns zero on success or if no request has been executed, the > + * status of the executed SAM request if that request failed, or %-EPROTO if > + * an unexpected response has been received. > + */ > +int ssam_ctrl_notif_display_off(struct ssam_controller *ctrl) > +{ > + int status; > + u8 response; > + > + ssam_dbg(ctrl, "pm: notifying display off\n"); > + > + status = ssam_retry(ssam_ssh_notif_display_off, ctrl, &response); > + if (status) > + return status; > + > + if (response != 0) { > + ssam_err(ctrl, "unexpected response from display-off notification: %#04x\n", > + response); > + return -EPROTO; > + } > + > + return 0; > +} > + > +/** > + * ssam_ctrl_notif_display_on() - Notify EC that the display has been turned on. > + * @ctrl: The controller. > + * > + * Notify the EC that the display has been turned back on and the driver has > + * exited its lower-power state. This notification is the counterpart to the > + * display-off notification sent via ssam_ctrl_notif_display_off() and will > + * reverse its effects, including resetting events to their default behavior. > + * > + * This function will only send the display-on notification command if display > + * notifications are supported by the EC. Currently all known devices support > + * these notifications. > + * > + * See ssam_ctrl_notif_display_off() for more details. > + * > + * Return: Returns zero on success or if no request has been executed, the > + * status of the executed SAM request if that request failed, or %-EPROTO if > + * an unexpected response has been received. > + */ > +int ssam_ctrl_notif_display_on(struct ssam_controller *ctrl) > +{ > + int status; > + u8 response; > + > + ssam_dbg(ctrl, "pm: notifying display on\n"); > + > + status = ssam_retry(ssam_ssh_notif_display_on, ctrl, &response); > + if (status) > + return status; > + > + if (response != 0) { > + ssam_err(ctrl, "unexpected response from display-on notification: %#04x\n", > + response); > + return -EPROTO; > + } > + > + return 0; > +} > + > +/** > + * ssam_ctrl_notif_d0_exit() - Notify EC that the driver/device exits the D0 > + * power state. > + * @ctrl: The controller > + * > + * Notifies the EC that the driver prepares to exit the D0 power state in > + * favor of a lower-power state. Exact effects of this function related to the > + * EC are currently unknown. > + * > + * This function will only send the D0-exit notification command if D0-state > + * notifications are supported by the EC. Only newer Surface generations > + * support these notifications. > + * > + * Use ssam_ctrl_notif_d0_entry() to reverse the effects of this function. > + * > + * Return: Returns zero on success or if no request has been executed, the > + * status of the executed SAM request if that request failed, or %-EPROTO if > + * an unexpected response has been received. > + */ > +int ssam_ctrl_notif_d0_exit(struct ssam_controller *ctrl) > +{ > + int status; > + u8 response; > + > + if (!ctrl->caps.d3_closes_handle) > + return 0; > + > + ssam_dbg(ctrl, "pm: notifying D0 exit\n"); > + > + status = ssam_retry(ssam_ssh_notif_d0_exit, ctrl, &response); > + if (status) > + return status; > + > + if (response != 0) { > + ssam_err(ctrl, "unexpected response from D0-exit notification: %#04x\n", > + response); > + return -EPROTO; > + } > + > + return 0; > +} > + > +/** > + * ssam_ctrl_notif_d0_entry() - Notify EC that the driver/device enters the D0 > + * power state. > + * @ctrl: The controller > + * > + * Notifies the EC that the driver has exited a lower-power state and entered > + * the D0 power state. Exact effects of this function related to the EC are > + * currently unknown. > + * > + * This function will only send the D0-entry notification command if D0-state > + * notifications are supported by the EC. Only newer Surface generations > + * support these notifications. > + * > + * See ssam_ctrl_notif_d0_exit() for more details. > + * > + * Return: Returns zero on success or if no request has been executed, the > + * status of the executed SAM request if that request failed, or %-EPROTO if > + * an unexpected response has been received. > + */ > +int ssam_ctrl_notif_d0_entry(struct ssam_controller *ctrl) > +{ > + int status; > + u8 response; > + > + if (!ctrl->caps.d3_closes_handle) > + return 0; > + > + ssam_dbg(ctrl, "pm: notifying D0 entry\n"); > + > + status = ssam_retry(ssam_ssh_notif_d0_entry, ctrl, &response); > + if (status) > + return status; > + > + if (response != 0) { > + ssam_err(ctrl, "unexpected response from D0-entry notification: %#04x\n", > + response); > + return -EPROTO; > + } > + > + return 0; > +} > + > + > +/* -- Top-level event registry interface. ----------------------------------- */ > + > +/** > + * ssam_notifier_register() - Register an event notifier. > + * @ctrl: The controller to register the notifier on. > + * @n: The event notifier to register. > + * > + * Register an event notifier and increment the usage counter of the > + * associated SAM event. If the event was previously not enabled, it will be > + * enabled during this call. > + * > + * Return: Returns zero on success, %-ENOSPC if there have already been > + * %INT_MAX notifiers for the event ID/type associated with the notifier block > + * registered, %-ENOMEM if the corresponding event entry could not be > + * allocated. If this is the first time that a notifier block is registered > + * for the specific associated event, returns the status of the event-enable > + * EC-command. > + */ > +int ssam_notifier_register(struct ssam_controller *ctrl, > + struct ssam_event_notifier *n) > +{ > + u16 rqid = ssh_tc_to_rqid(n->event.id.target_category); > + struct ssam_nf_refcount_entry *entry; > + struct ssam_nf_head *nf_head; > + struct ssam_nf *nf; > + int status; > + > + if (!ssh_rqid_is_event(rqid)) > + return -EINVAL; > + > + nf = &ctrl->cplt.event.notif; > + nf_head = &nf->head[ssh_rqid_to_event(rqid)]; > + > + mutex_lock(&nf->lock); > + > + entry = ssam_nf_refcount_inc(nf, n->event.reg, n->event.id); > + if (IS_ERR(entry)) { > + mutex_unlock(&nf->lock); > + return PTR_ERR(entry); > + } > + > + ssam_dbg(ctrl, "enabling event (reg: %#04x, tc: %#04x, iid: %#04x, rc: %d)\n", > + n->event.reg.target_category, n->event.id.target_category, > + n->event.id.instance, entry->refcount); > + > + status = ssam_nfblk_insert(nf_head, &n->base); > + if (status) { > + entry = ssam_nf_refcount_dec(nf, n->event.reg, n->event.id); > + if (entry->refcount == 0) > + kfree(entry); > + > + mutex_unlock(&nf->lock); > + return status; > + } > + > + if (entry->refcount == 1) { > + status = ssam_ssh_event_enable(ctrl, n->event.reg, n->event.id, > + n->event.flags); > + if (status) { > + ssam_nfblk_remove(&n->base); > + kfree(ssam_nf_refcount_dec(nf, n->event.reg, n->event.id)); > + mutex_unlock(&nf->lock); > + synchronize_srcu(&nf_head->srcu); > + return status; > + } > + > + entry->flags = n->event.flags; > + > + } else if (entry->flags != n->event.flags) { > + ssam_warn(ctrl, > + "inconsistent flags when enabling event: got %#04x, expected %#04x (reg: %#04x, tc: %#04x, iid: %#04x)\n", > + n->event.flags, entry->flags, n->event.reg.target_category, > + n->event.id.target_category, n->event.id.instance); > + } > + > + mutex_unlock(&nf->lock); > + return 0; > +} > +EXPORT_SYMBOL_GPL(ssam_notifier_register); > + > +/** > + * ssam_notifier_unregister() - Unregister an event notifier. > + * @ctrl: The controller the notifier has been registered on. > + * @n: The event notifier to unregister. > + * > + * Unregister an event notifier and decrement the usage counter of the > + * associated SAM event. If the usage counter reaches zero, the event will be > + * disabled. > + * > + * Return: Returns zero on success, %-ENOENT if the given notifier block has > + * not been registered on the controller. If the given notifier block was the > + * last one associated with its specific event, returns the status of the > + * event-disable EC-command. > + */ > +int ssam_notifier_unregister(struct ssam_controller *ctrl, > + struct ssam_event_notifier *n) > +{ > + u16 rqid = ssh_tc_to_rqid(n->event.id.target_category); > + struct ssam_nf_refcount_entry *entry; > + struct ssam_nf_head *nf_head; > + struct ssam_nf *nf; > + int status = 0; > + > + if (!ssh_rqid_is_event(rqid)) > + return -EINVAL; > + > + nf = &ctrl->cplt.event.notif; > + nf_head = &nf->head[ssh_rqid_to_event(rqid)]; > + > + mutex_lock(&nf->lock); > + > + if (!ssam_nfblk_find(nf_head, &n->base)) { > + mutex_unlock(&nf->lock); > + return -ENOENT; > + } > + > + entry = ssam_nf_refcount_dec(nf, n->event.reg, n->event.id); > + if (WARN_ON(!entry)) { > + /* > + * If this does not return an entry, there's a logic error > + * somewhere: The notifier block is registered, but the event > + * refcount entry is not there. Remove the notifier block > + * anyways. > + */ > + status = -ENOENT; > + goto remove; > + } > + > + ssam_dbg(ctrl, "disabling event (reg: %#04x, tc: %#04x, iid: %#04x, rc: %d)\n", > + n->event.reg.target_category, n->event.id.target_category, > + n->event.id.instance, entry->refcount); > + > + if (entry->flags != n->event.flags) { > + ssam_warn(ctrl, > + "inconsistent flags when disabling event: got %#04x, expected %#04x (reg: %#04x, tc: %#04x, iid: %#04x)\n", > + n->event.flags, entry->flags, n->event.reg.target_category, > + n->event.id.target_category, n->event.id.instance); > + } > + > + if (entry->refcount == 0) { > + status = ssam_ssh_event_disable(ctrl, n->event.reg, n->event.id, > + n->event.flags); > + kfree(entry); > + } > + > +remove: > + ssam_nfblk_remove(&n->base); > + mutex_unlock(&nf->lock); > + synchronize_srcu(&nf_head->srcu); > + > + return status; > +} > +EXPORT_SYMBOL_GPL(ssam_notifier_unregister); > + > +/** > + * ssam_notifier_disable_registered() - Disable events for all registered > + * notifiers. > + * @ctrl: The controller for which to disable the notifiers/events. > + * > + * Disables events for all currently registered notifiers. In case of an error > + * (EC command failing), all previously disabled events will be restored and > + * the error code returned. > + * > + * This function is intended to disable all events prior to hibernation entry. > + * See ssam_notifier_restore_registered() to restore/re-enable all events > + * disabled with this function. > + * > + * Note that this function will not disable events for notifiers registered > + * after calling this function. It should thus be made sure that no new > + * notifiers are going to be added after this call and before the corresponding > + * call to ssam_notifier_restore_registered(). > + * > + * Return: Returns zero on success. In case of failure returns the error code > + * returned by the failed EC command to disable an event. > + */ > +int ssam_notifier_disable_registered(struct ssam_controller *ctrl) > +{ > + struct ssam_nf *nf = &ctrl->cplt.event.notif; > + struct rb_node *n; > + int status; > + > + mutex_lock(&nf->lock); > + for (n = rb_first(&nf->refcount); n; n = rb_next(n)) { > + struct ssam_nf_refcount_entry *e; > + > + e = rb_entry(n, struct ssam_nf_refcount_entry, node); > + status = ssam_ssh_event_disable(ctrl, e->key.reg, > + e->key.id, e->flags); > + if (status) > + goto err; > + } > + mutex_unlock(&nf->lock); > + > + return 0; > + > +err: > + for (n = rb_prev(n); n; n = rb_prev(n)) { > + struct ssam_nf_refcount_entry *e; > + > + e = rb_entry(n, struct ssam_nf_refcount_entry, node); > + ssam_ssh_event_enable(ctrl, e->key.reg, e->key.id, e->flags); > + } > + mutex_unlock(&nf->lock); > + > + return status; > +} > + > +/** > + * ssam_notifier_restore_registered() - Restore/re-enable events for all > + * registered notifiers. > + * @ctrl: The controller for which to restore the notifiers/events. > + * > + * Restores/re-enables all events for which notifiers have been registered on > + * the given controller. In case of a failure, the error is logged and the > + * function continues to try and enable the remaining events. > + * > + * This function is intended to restore/re-enable all registered events after > + * hibernation. See ssam_notifier_disable_registered() for the counter part > + * disabling the events and more details. > + */ > +void ssam_notifier_restore_registered(struct ssam_controller *ctrl) > +{ > + struct ssam_nf *nf = &ctrl->cplt.event.notif; > + struct rb_node *n; > + > + mutex_lock(&nf->lock); > + for (n = rb_first(&nf->refcount); n; n = rb_next(n)) { > + struct ssam_nf_refcount_entry *e; > + > + e = rb_entry(n, struct ssam_nf_refcount_entry, node); > + > + /* Ignore errors, will get logged in call. */ > + ssam_ssh_event_enable(ctrl, e->key.reg, e->key.id, e->flags); > + } > + mutex_unlock(&nf->lock); > +} > + > +/** > + * ssam_notifier_is_empty() - Check if there are any registered notifiers. > + * @ctrl: The controller to check on. > + * > + * Return: Returns %true if there are currently no notifiers registered on the > + * controller, %false otherwise. > + */ > +static bool ssam_notifier_is_empty(struct ssam_controller *ctrl) > +{ > + struct ssam_nf *nf = &ctrl->cplt.event.notif; > + bool result; > + > + mutex_lock(&nf->lock); > + result = ssam_nf_refcount_empty(nf); > + mutex_unlock(&nf->lock); > + > + return result; > +} > + > +/** > + * ssam_notifier_unregister_all() - Unregister all currently registered > + * notifiers. > + * @ctrl: The controller to unregister the notifiers on. > + * > + * Unregisters all currently registered notifiers. This function is used to > + * ensure that all notifiers will be unregistered and associated > + * entries/resources freed when the controller is being shut down. > + */ > +static void ssam_notifier_unregister_all(struct ssam_controller *ctrl) > +{ > + struct ssam_nf *nf = &ctrl->cplt.event.notif; > + struct ssam_nf_refcount_entry *e, *n; > + > + mutex_lock(&nf->lock); > + rbtree_postorder_for_each_entry_safe(e, n, &nf->refcount, node) { > + /* Ignore errors, will get logged in call. */ > + ssam_ssh_event_disable(ctrl, e->key.reg, e->key.id, e->flags); > + kfree(e); > + } > + nf->refcount = RB_ROOT; > + mutex_unlock(&nf->lock); > +} > + > + > +/* -- Wakeup IRQ. ----------------------------------------------------------- */ > + > +static irqreturn_t ssam_irq_handle(int irq, void *dev_id) > +{ > + struct ssam_controller *ctrl = dev_id; > + > + ssam_dbg(ctrl, "pm: wake irq triggered\n"); > + > + /* > + * Note: Proper wakeup detection is currently unimplemented. > + * When the EC is in display-off or any other non-D0 state, it > + * does not send events/notifications to the host. Instead it > + * signals that there are events available via the wakeup IRQ. > + * This driver is responsible for calling back to the EC to > + * release these events one-by-one. > + * > + * This IRQ should not cause a full system resume by its own. > + * Instead, events should be handled by their respective subsystem > + * drivers, which in turn should signal whether a full system > + * resume should be performed. > + * > + * TODO: Send GPIO callback command repeatedly to EC until callback > + * returns 0x00. Return flag of callback is "has more events". > + * Each time the command is sent, one event is "released". Once > + * all events have been released (return = 0x00), the GPIO is > + * re-armed. Detect wakeup events during this process, go back to > + * sleep if no wakeup event has been received. > + */ > + > + return IRQ_HANDLED; > +} > + > +/** > + * ssam_irq_setup() - Set up SAM EC wakeup-GPIO interrupt. > + * @ctrl: The controller for which the IRQ should be set up. > + * > + * Set up an IRQ for the wakeup-GPIO pin of the SAM EC. This IRQ can be used > + * to wake the device from a low power state. > + * > + * Note that this IRQ can only be triggered while the EC is in the display-off > + * state. In this state, events are not sent to the host in the usual way. > + * Instead the wakeup-GPIO gets pulled to "high" as long as there are pending > + * events and these events need to be released one-by-one via the GPIO > + * callback request, either until there are no events left and the GPIO is > + * reset, or all at once by transitioning the EC out of the display-off state, > + * which will also clear the GPIO. > + * > + * Not all events, however, should trigger a full system wakeup. Instead the > + * driver should, if necessary, inspect and forward each event to the > + * corresponding subsystem, which in turn should decide if the system needs to > + * be woken up. This logic has not been implemented yet, thus wakeup by this > + * IRQ should be disabled by default to avoid spurious wake-ups, caused, for > + * example, by the remaining battery percentage changing. Refer to comments in > + * this function and comments in the corresponding IRQ handler for more > + * details on how this should be implemented. > + * > + * See also ssam_ctrl_notif_display_off() and ssam_ctrl_notif_display_off() > + * for functions to transition the EC into and out of the display-off state as > + * well as more details on it. > + * > + * The IRQ is disabled by default and has to be enabled before it can wake up > + * the device from suspend via ssam_irq_arm_for_wakeup(). On teardown, the IRQ > + * should be freed via ssam_irq_free(). > + */ > +int ssam_irq_setup(struct ssam_controller *ctrl) > +{ > + struct device *dev = ssam_controller_device(ctrl); > + struct gpio_desc *gpiod; > + int irq; > + int status; > + > + /* > + * The actual GPIO interrupt is declared in ACPI as TRIGGER_HIGH. > + * However, the GPIO line only gets reset by sending the GPIO callback > + * command to SAM (or alternatively the display-on notification). As > + * proper handling for this interrupt is not implemented yet, leaving > + * the IRQ at TRIGGER_HIGH would cause an IRQ storm (as the callback > + * never gets sent and thus the line never gets reset). To avoid this, > + * mark the IRQ as TRIGGER_RISING for now, only creating a single > + * interrupt, and let the SAM resume callback during the controller > + * resume process clear it. > + */ > + const int irqf = IRQF_SHARED | IRQF_ONESHOT | IRQF_TRIGGER_RISING; > + > + gpiod = gpiod_get(dev, "ssam_wakeup-int", GPIOD_ASIS); > + if (IS_ERR(gpiod)) > + return PTR_ERR(gpiod); > + > + irq = gpiod_to_irq(gpiod); > + gpiod_put(gpiod); > + > + if (irq < 0) > + return irq; > + > + status = request_threaded_irq(irq, NULL, ssam_irq_handle, irqf, > + "ssam_wakeup", ctrl); > + if (status) > + return status; > + > + ctrl->irq.num = irq; > + disable_irq(ctrl->irq.num); > + return 0; > +} > + > +/** > + * ssam_irq_free() - Free SAM EC wakeup-GPIO interrupt. > + * @ctrl: The controller for which the IRQ should be freed. > + * > + * Free the wakeup-GPIO IRQ previously set-up via ssam_irq_setup(). > + */ > +void ssam_irq_free(struct ssam_controller *ctrl) > +{ > + free_irq(ctrl->irq.num, ctrl); > + ctrl->irq.num = -1; > +} > + > +/** > + * ssam_irq_arm_for_wakeup() - Arm the EC IRQ for wakeup, if enabled. > + * @ctrl: The controller for which the IRQ should be armed. > + * > + * Sets up the IRQ so that it can be used to wake the device. Specifically, > + * this function enables the irq and then, if the device is allowed to wake up > + * the system, calls enable_irq_wake(). See ssam_irq_disarm_wakeup() for the > + * corresponding function to disable the IRQ. > + * > + * This function is intended to arm the IRQ before entering S2idle suspend. > + * > + * Note: calls to ssam_irq_arm_for_wakeup() and ssam_irq_disarm_wakeup() must > + * be balanced. > + */ > +int ssam_irq_arm_for_wakeup(struct ssam_controller *ctrl) > +{ > + struct device *dev = ssam_controller_device(ctrl); > + int status; > + > + enable_irq(ctrl->irq.num); > + if (device_may_wakeup(dev)) { > + status = enable_irq_wake(ctrl->irq.num); > + if (status) { > + ssam_err(ctrl, "failed to enable wake IRQ: %d\n", status); > + disable_irq(ctrl->irq.num); > + return status; > + } > + > + ctrl->irq.wakeup_enabled = true; > + } else { > + ctrl->irq.wakeup_enabled = false; > + } > + > + return 0; > +} > + > +/** > + * ssam_irq_disarm_wakeup() - Disarm the wakeup IRQ. > + * @ctrl: The controller for which the IRQ should be disarmed. > + * > + * Disarm the IRQ previously set up for wake via ssam_irq_arm_for_wakeup(). > + * > + * This function is intended to disarm the IRQ after exiting S2idle suspend. > + * > + * Note: calls to ssam_irq_arm_for_wakeup() and ssam_irq_disarm_wakeup() must > + * be balanced. > + */ > +void ssam_irq_disarm_wakeup(struct ssam_controller *ctrl) > +{ > + int status; > + > + if (ctrl->irq.wakeup_enabled) { > + status = disable_irq_wake(ctrl->irq.num); > + if (status) > + ssam_err(ctrl, "failed to disable wake IRQ: %d\n", status); > + > + ctrl->irq.wakeup_enabled = false; > + } > + disable_irq(ctrl->irq.num); > +} > diff --git a/drivers/platform/surface/aggregator/controller.h b/drivers/platform/surface/aggregator/controller.h > new file mode 100644 > index 000000000000..5ee9e966f1d7 > --- /dev/null > +++ b/drivers/platform/surface/aggregator/controller.h > @@ -0,0 +1,276 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Main SSAM/SSH controller structure and functionality. > + * > + * Copyright (C) 2019-2020 Maximilian Luz <luzmaximilian@xxxxxxxxx> > + */ > + > +#ifndef _SURFACE_AGGREGATOR_CONTROLLER_H > +#define _SURFACE_AGGREGATOR_CONTROLLER_H > + > +#include <linux/kref.h> > +#include <linux/list.h> > +#include <linux/mutex.h> > +#include <linux/rbtree.h> > +#include <linux/rwsem.h> > +#include <linux/serdev.h> > +#include <linux/spinlock.h> > +#include <linux/srcu.h> > +#include <linux/types.h> > +#include <linux/workqueue.h> > + > +#include <linux/surface_aggregator/controller.h> > +#include <linux/surface_aggregator/serial_hub.h> > + > +#include "ssh_request_layer.h" > + > + > +/* -- Safe counters. -------------------------------------------------------- */ > + > +/** > + * struct ssh_seq_counter - Safe counter for SSH sequence IDs. > + * @value: The current counter value. > + */ > +struct ssh_seq_counter { > + u8 value; > +}; > + > +/** > + * struct ssh_rqid_counter - Safe counter for SSH request IDs. > + * @value: The current counter value. > + */ > +struct ssh_rqid_counter { > + u16 value; > +}; > + > + > +/* -- Event/notification system. -------------------------------------------- */ > + > +/** > + * struct ssam_nf_head - Notifier head for SSAM events. > + * @srcu: The SRCU struct for synchronization. > + * @head: List-head for notifier blocks registered under this head. > + */ > +struct ssam_nf_head { > + struct srcu_struct srcu; > + struct list_head head; > +}; > + > +/** > + * struct ssam_nf - Notifier callback- and activation-registry for SSAM events. > + * @lock: Lock guarding (de-)registration of notifier blocks. Note: This > + * lock does not need to be held for notifier calls, only > + * registration and deregistration. > + * @refcount: The root of the RB-tree used for reference-counting enabled > + * events/notifications. > + * @head: The list of notifier heads for event/notification callbacks. > + */ > +struct ssam_nf { > + struct mutex lock; > + struct rb_root refcount; > + struct ssam_nf_head head[SSH_NUM_EVENTS]; > +}; > + > + > +/* -- Event/async request completion system. -------------------------------- */ > + > +struct ssam_cplt; > + > +/** > + * struct ssam_event_item - Struct for event queuing and completion. > + * @node: The node in the queue. > + * @rqid: The request ID of the event. > + * @event: Actual event data. > + */ > +struct ssam_event_item { > + struct list_head node; > + u16 rqid; > + > + struct ssam_event event; /* must be last */ > +}; > + > +/** > + * struct ssam_event_queue - Queue for completing received events. > + * @cplt: Reference to the completion system on which this queue is active. > + * @lock: The lock for any operation on the queue. > + * @head: The list-head of the queue. > + * @work: The &struct work_struct performing completion work for this queue. > + */ > +struct ssam_event_queue { > + struct ssam_cplt *cplt; > + > + spinlock_t lock; > + struct list_head head; > + struct work_struct work; > +}; > + > +/** > + * struct ssam_event_target - Set of queues for a single SSH target ID. > + * @queue: The array of queues, one queue per event ID. > + */ > +struct ssam_event_target { > + struct ssam_event_queue queue[SSH_NUM_EVENTS]; > +}; > + > +/** > + * struct ssam_cplt - SSAM event/async request completion system. > + * @dev: The device with which this system is associated. Only used > + * for logging. > + * @wq: The &struct workqueue_struct on which all completion work > + * items are queued. > + * @event: Event completion management. > + * @event.target: Array of &struct ssam_event_target, one for each target. > + * @event.notif: Notifier callbacks and event activation reference counting. > + */ > +struct ssam_cplt { > + struct device *dev; > + struct workqueue_struct *wq; > + > + struct { > + struct ssam_event_target target[SSH_NUM_TARGETS]; > + struct ssam_nf notif; > + } event; > +}; > + > + > +/* -- Main SSAM device structures. ------------------------------------------ */ > + > +/** > + * enum ssam_controller_state - State values for &struct ssam_controller. > + * @SSAM_CONTROLLER_UNINITIALIZED: > + * The controller has not been initialized yet or has been deinitialized. > + * @SSAM_CONTROLLER_INITIALIZED: > + * The controller is initialized, but has not been started yet. > + * @SSAM_CONTROLLER_STARTED: > + * The controller has been started and is ready to use. > + * @SSAM_CONTROLLER_STOPPED: > + * The controller has been stopped. > + * @SSAM_CONTROLLER_SUSPENDED: > + * The controller has been suspended. > + */ > +enum ssam_controller_state { > + SSAM_CONTROLLER_UNINITIALIZED, > + SSAM_CONTROLLER_INITIALIZED, > + SSAM_CONTROLLER_STARTED, > + SSAM_CONTROLLER_STOPPED, > + SSAM_CONTROLLER_SUSPENDED, > +}; > + > +/** > + * struct ssam_controller_caps - Controller device capabilities. > + * @ssh_power_profile: SSH power profile. > + * @ssh_buffer_size: SSH driver UART buffer size. > + * @screen_on_sleep_idle_timeout: SAM UART screen-on sleep idle timeout. > + * @screen_off_sleep_idle_timeout: SAM UART screen-off sleep idle timeout. > + * @d3_closes_handle: SAM closes UART handle in D3. > + * > + * Controller and SSH device capabilities found in ACPI. > + */ > +struct ssam_controller_caps { > + u32 ssh_power_profile; > + u32 ssh_buffer_size; > + u32 screen_on_sleep_idle_timeout; > + u32 screen_off_sleep_idle_timeout; > + u32 d3_closes_handle:1; > +}; > + > +/** > + * struct ssam_controller - SSAM controller device. > + * @kref: Reference count of the controller. > + * @lock: Main lock for the controller, used to guard state changes. > + * @state: Controller state. > + * @rtl: Request transport layer for SSH I/O. > + * @cplt: Completion system for SSH/SSAM events and asynchronous requests. > + * @counter: Safe SSH message ID counters. > + * @counter.seq: Sequence ID counter. > + * @counter.rqid: Request ID counter. > + * @irq: Wakeup IRQ resources. > + * @irq.num: The wakeup IRQ number. > + * @irq.wakeup_enabled: Whether wakeup by IRQ is enabled during suspend. > + * @caps: The controller device capabilities. > + */ > +struct ssam_controller { > + struct kref kref; > + > + struct rw_semaphore lock; > + enum ssam_controller_state state; > + > + struct ssh_rtl rtl; > + struct ssam_cplt cplt; > + > + struct { > + struct ssh_seq_counter seq; > + struct ssh_rqid_counter rqid; > + } counter; > + > + struct { > + int num; > + bool wakeup_enabled; > + } irq; > + > + struct ssam_controller_caps caps; > +}; > + > +#define to_ssam_controller(ptr, member) \ > + container_of(ptr, struct ssam_controller, member) > + > +#define ssam_dbg(ctrl, fmt, ...) rtl_dbg(&(ctrl)->rtl, fmt, ##__VA_ARGS__) > +#define ssam_info(ctrl, fmt, ...) rtl_info(&(ctrl)->rtl, fmt, ##__VA_ARGS__) > +#define ssam_warn(ctrl, fmt, ...) rtl_warn(&(ctrl)->rtl, fmt, ##__VA_ARGS__) > +#define ssam_err(ctrl, fmt, ...) rtl_err(&(ctrl)->rtl, fmt, ##__VA_ARGS__) > + > +/** > + * ssam_controller_receive_buf() - Provide input-data to the controller. > + * @ctrl: The controller. > + * @buf: The input buffer. > + * @n: The number of bytes in the input buffer. > + * > + * Provide input data to be evaluated by the controller, which has been > + * received via the lower-level transport. > + * > + * Return: Returns the number of bytes consumed, or, if the packet transport > + * layer of the controller has been shut down, %-ESHUTDOWN. > + */ > +static inline > +int ssam_controller_receive_buf(struct ssam_controller *ctrl, > + const unsigned char *buf, size_t n) > +{ > + return ssh_ptl_rx_rcvbuf(&ctrl->rtl.ptl, buf, n); > +} > + > +/** > + * ssam_controller_write_wakeup() - Notify the controller that the underlying > + * device has space available for data to be written. > + * @ctrl: The controller. > + */ > +static inline void ssam_controller_write_wakeup(struct ssam_controller *ctrl) > +{ > + ssh_ptl_tx_wakeup_transfer(&ctrl->rtl.ptl); > +} > + > +int ssam_controller_init(struct ssam_controller *ctrl, struct serdev_device *s); > +int ssam_controller_start(struct ssam_controller *ctrl); > +void ssam_controller_shutdown(struct ssam_controller *ctrl); > +void ssam_controller_destroy(struct ssam_controller *ctrl); > + > +int ssam_notifier_disable_registered(struct ssam_controller *ctrl); > +void ssam_notifier_restore_registered(struct ssam_controller *ctrl); > + > +int ssam_irq_setup(struct ssam_controller *ctrl); > +void ssam_irq_free(struct ssam_controller *ctrl); > +int ssam_irq_arm_for_wakeup(struct ssam_controller *ctrl); > +void ssam_irq_disarm_wakeup(struct ssam_controller *ctrl); > + > +void ssam_controller_lock(struct ssam_controller *c); > +void ssam_controller_unlock(struct ssam_controller *c); > + > +int ssam_get_firmware_version(struct ssam_controller *ctrl, u32 *version); > +int ssam_ctrl_notif_display_off(struct ssam_controller *ctrl); > +int ssam_ctrl_notif_display_on(struct ssam_controller *ctrl); > +int ssam_ctrl_notif_d0_exit(struct ssam_controller *ctrl); > +int ssam_ctrl_notif_d0_entry(struct ssam_controller *ctrl); > + > +int ssam_controller_suspend(struct ssam_controller *ctrl); > +int ssam_controller_resume(struct ssam_controller *ctrl); > + > +#endif /* _SURFACE_AGGREGATOR_CONTROLLER_H */ > diff --git a/drivers/platform/surface/aggregator/core.c b/drivers/platform/surface/aggregator/core.c > new file mode 100644 > index 000000000000..ec6c7f40ad36 > --- /dev/null > +++ b/drivers/platform/surface/aggregator/core.c > @@ -0,0 +1,781 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Surface Serial Hub (SSH) driver for communication with the Surface/System > + * Aggregator Module (SSAM/SAM). > + * > + * Provides access to a SAM-over-SSH connected EC via a controller device. > + * Handles communication via requests as well as enabling, disabling, and > + * relaying of events. > + * > + * Copyright (C) 2019-2020 Maximilian Luz <luzmaximilian@xxxxxxxxx> > + */ > + > +#include <linux/acpi.h> > +#include <linux/atomic.h> > +#include <linux/completion.h> > +#include <linux/gpio/consumer.h> > +#include <linux/kernel.h> > +#include <linux/kref.h> > +#include <linux/module.h> > +#include <linux/pm.h> > +#include <linux/serdev.h> > +#include <linux/sysfs.h> > + > +#include <linux/surface_aggregator/controller.h> > +#include "controller.h" > + > + > +/* -- Static controller reference. ------------------------------------------ */ > + > +/* > + * Main controller reference. The corresponding lock must be held while > + * accessing (reading/writing) the reference. > + */ > +static struct ssam_controller *__ssam_controller; > +static DEFINE_SPINLOCK(__ssam_controller_lock); > + > +/** > + * ssam_get_controller() - Get reference to SSAM controller. > + * > + * Returns a reference to the SSAM controller of the system or %NULL if there > + * is none, it hasn't been set up yet, or it has already been unregistered. > + * This function automatically increments the reference count of the > + * controller, thus the calling party must ensure that ssam_controller_put() > + * is called when it doesn't need the controller any more. > + */ > +struct ssam_controller *ssam_get_controller(void) > +{ > + struct ssam_controller *ctrl; > + > + spin_lock(&__ssam_controller_lock); > + > + ctrl = __ssam_controller; > + if (!ctrl) > + goto out; > + > + if (WARN_ON(!kref_get_unless_zero(&ctrl->kref))) > + ctrl = NULL; > + > +out: > + spin_unlock(&__ssam_controller_lock); > + return ctrl; > +} > +EXPORT_SYMBOL_GPL(ssam_get_controller); > + > +/** > + * ssam_try_set_controller() - Try to set the main controller reference. > + * @ctrl: The controller to which the reference should point. > + * > + * Set the main controller reference to the given pointer if the reference > + * hasn't been set already. > + * > + * Return: Returns zero on success or %-EEXIST if the reference has already > + * been set. > + */ > +static int ssam_try_set_controller(struct ssam_controller *ctrl) > +{ > + int status = 0; > + > + spin_lock(&__ssam_controller_lock); > + if (!__ssam_controller) > + __ssam_controller = ctrl; > + else > + status = -EEXIST; > + spin_unlock(&__ssam_controller_lock); > + > + return status; > +} > + > +/** > + * ssam_clear_controller() - Remove/clear the main controller reference. > + * > + * Clears the main controller reference, i.e. sets it to %NULL. This function > + * should be called before the controller is shut down. > + */ > +static void ssam_clear_controller(void) > +{ > + spin_lock(&__ssam_controller_lock); > + __ssam_controller = NULL; > + spin_unlock(&__ssam_controller_lock); > +} > + > +/** > + * ssam_client_link() - Link an arbitrary client device to the controller. > + * @c: The controller to link to. > + * @client: The client device. > + * > + * Link an arbitrary client device to the controller by creating a device link > + * between it as consumer and the controller device as provider. This function > + * can be used for non-SSAM devices (or SSAM devices not registered as child > + * under the controller) to guarantee that the controller is valid for as long > + * as the driver of the client device is bound, and that proper suspend and > + * resume ordering is guaranteed. > + * > + * The device link does not have to be destructed manually. It is removed > + * automatically once the driver of the client device unbinds. > + * > + * Return: Returns zero on success, %-ENODEV if the controller is not ready or > + * going to be removed soon, or %-ENOMEM if the device link could not be > + * created for other reasons. > + */ > +int ssam_client_link(struct ssam_controller *c, struct device *client) > +{ > + const u32 flags = DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_CONSUMER; > + struct device_link *link; > + struct device *ctrldev; > + > + ssam_controller_statelock(c); > + > + if (c->state != SSAM_CONTROLLER_STARTED) { > + ssam_controller_stateunlock(c); > + return -ENODEV; > + } > + > + ctrldev = ssam_controller_device(c); > + if (!ctrldev) { > + ssam_controller_stateunlock(c); > + return -ENODEV; > + } > + > + link = device_link_add(client, ctrldev, flags); > + if (!link) { > + ssam_controller_stateunlock(c); > + return -ENOMEM; > + } > + > + /* > + * Return -ENODEV if supplier driver is on its way to be removed. In > + * this case, the controller won't be around for much longer and the > + * device link is not going to save us any more, as unbinding is > + * already in progress. > + */ > + if (READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND) { > + ssam_controller_stateunlock(c); > + return -ENODEV; > + } > + > + ssam_controller_stateunlock(c); > + return 0; > +} > +EXPORT_SYMBOL_GPL(ssam_client_link); > + > +/** > + * ssam_client_bind() - Bind an arbitrary client device to the controller. > + * @client: The client device. > + * > + * Link an arbitrary client device to the controller by creating a device link > + * between it as consumer and the main controller device as provider. This > + * function can be used for non-SSAM devices to guarantee that the controller > + * returned by this function is valid for as long as the driver of the client > + * device is bound, and that proper suspend and resume ordering is guaranteed. > + * > + * This function does essentially the same as ssam_client_link(), except that > + * it first fetches the main controller reference, then creates the link, and > + * finally returns this reference. Note that this function does not increment > + * the reference counter of the controller, as, due to the link, the > + * controller lifetime is assured as long as the driver of the client device > + * is bound. > + * > + * It is not valid to use the controller reference obtained by this method > + * outside of the driver bound to the client device at the time of calling > + * this function, without first incrementing the reference count of the > + * controller via ssam_controller_get(). Even after doing this, care must be > + * taken that requests are only submitted and notifiers are only > + * (un-)registered when the controller is active and not suspended. In other > + * words: The device link only lives as long as the client driver is bound and > + * any guarantees enforced by this link (e.g. active controller state) can > + * only be relied upon as long as this link exists and may need to be enforced > + * in other ways afterwards. > + * > + * The created device link does not have to be destructed manually. It is > + * removed automatically once the driver of the client device unbinds. > + * > + * Return: Returns the controller on success, an error pointer with %-ENODEV > + * if the controller is not present, not ready or going to be removed soon, or > + * %-ENOMEM if the device link could not be created for other reasons. > + */ > +struct ssam_controller *ssam_client_bind(struct device *client) > +{ > + struct ssam_controller *c; > + int status; > + > + c = ssam_get_controller(); > + if (!c) > + return ERR_PTR(-ENODEV); > + > + status = ssam_client_link(c, client); > + > + /* > + * Note that we can drop our controller reference in both success and > + * failure cases: On success, we have bound the controller lifetime > + * inherently to the client driver lifetime, i.e. it the controller is > + * now guaranteed to outlive the client driver. On failure, we're not > + * going to use the controller any more. > + */ > + ssam_controller_put(c); > + > + return status >= 0 ? c : ERR_PTR(status); > +} > +EXPORT_SYMBOL_GPL(ssam_client_bind); > + > + > +/* -- Glue layer (serdev_device -> ssam_controller). ------------------------ */ > + > +static int ssam_receive_buf(struct serdev_device *dev, const unsigned char *buf, > + size_t n) > +{ > + struct ssam_controller *ctrl; > + > + ctrl = serdev_device_get_drvdata(dev); > + return ssam_controller_receive_buf(ctrl, buf, n); > +} > + > +static void ssam_write_wakeup(struct serdev_device *dev) > +{ > + ssam_controller_write_wakeup(serdev_device_get_drvdata(dev)); > +} > + > +static const struct serdev_device_ops ssam_serdev_ops = { > + .receive_buf = ssam_receive_buf, > + .write_wakeup = ssam_write_wakeup, > +}; > + > + > +/* -- SysFS and misc. ------------------------------------------------------- */ > + > +static int ssam_log_firmware_version(struct ssam_controller *ctrl) > +{ > + u32 version, a, b, c; > + int status; > + > + status = ssam_get_firmware_version(ctrl, &version); > + if (status) > + return status; > + > + a = (version >> 24) & 0xff; > + b = ((version >> 8) & 0xffff); > + c = version & 0xff; > + > + ssam_info(ctrl, "SAM firmware version: %u.%u.%u\n", a, b, c); > + return 0; > +} > + > +static ssize_t firmware_version_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct ssam_controller *ctrl = dev_get_drvdata(dev); > + u32 version, a, b, c; > + int status; > + > + status = ssam_get_firmware_version(ctrl, &version); > + if (status < 0) > + return status; > + > + a = (version >> 24) & 0xff; > + b = ((version >> 8) & 0xffff); > + c = version & 0xff; > + > + return sysfs_emit(buf, "%u.%u.%u\n", a, b, c); > +} > +static DEVICE_ATTR_RO(firmware_version); > + > +static struct attribute *ssam_sam_attrs[] = { > + &dev_attr_firmware_version.attr, > + NULL > +}; > + > +static const struct attribute_group ssam_sam_group = { > + .name = "sam", > + .attrs = ssam_sam_attrs, > +}; > + > + > +/* -- ACPI based device setup. ---------------------------------------------- */ > + > +static acpi_status ssam_serdev_setup_via_acpi_crs(struct acpi_resource *rsc, > + void *ctx) > +{ > + struct serdev_device *serdev = ctx; > + struct acpi_resource_common_serialbus *serial; > + struct acpi_resource_uart_serialbus *uart; > + bool flow_control; > + int status = 0; > + > + if (rsc->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) > + return AE_OK; > + > + serial = &rsc->data.common_serial_bus; > + if (serial->type != ACPI_RESOURCE_SERIAL_TYPE_UART) > + return AE_OK; > + > + uart = &rsc->data.uart_serial_bus; > + > + /* Set up serdev device. */ > + serdev_device_set_baudrate(serdev, uart->default_baud_rate); > + > + /* serdev currently only supports RTSCTS flow control. */ > + if (uart->flow_control & (~((u8)ACPI_UART_FLOW_CONTROL_HW))) { > + dev_warn(&serdev->dev, "setup: unsupported flow control (value: %#04x)\n", > + uart->flow_control); > + } > + > + /* Set RTSCTS flow control. */ > + flow_control = uart->flow_control & ACPI_UART_FLOW_CONTROL_HW; > + serdev_device_set_flow_control(serdev, flow_control); > + > + /* serdev currently only supports EVEN/ODD parity. */ > + switch (uart->parity) { > + case ACPI_UART_PARITY_NONE: > + status = serdev_device_set_parity(serdev, SERDEV_PARITY_NONE); > + break; > + case ACPI_UART_PARITY_EVEN: > + status = serdev_device_set_parity(serdev, SERDEV_PARITY_EVEN); > + break; > + case ACPI_UART_PARITY_ODD: > + status = serdev_device_set_parity(serdev, SERDEV_PARITY_ODD); > + break; > + default: > + dev_warn(&serdev->dev, "setup: unsupported parity (value: %#04x)\n", > + uart->parity); > + break; > + } > + > + if (status) { > + dev_err(&serdev->dev, "setup: failed to set parity (value: %#04x, error: %d)\n", > + uart->parity, status); > + return AE_ERROR; > + } > + > + /* We've found the resource and are done. */ > + return AE_CTRL_TERMINATE; > +} > + > +static acpi_status ssam_serdev_setup_via_acpi(acpi_handle handle, > + struct serdev_device *serdev) > +{ > + return acpi_walk_resources(handle, METHOD_NAME__CRS, > + ssam_serdev_setup_via_acpi_crs, serdev); > +} > + > + > +/* -- Power management. ----------------------------------------------------- */ > + > +static void ssam_serial_hub_shutdown(struct device *dev) > +{ > + struct ssam_controller *c = dev_get_drvdata(dev); > + int status; > + > + /* > + * Try to disable notifiers, signal display-off and D0-exit, ignore any > + * errors. > + * > + * Note: It has not been established yet if this is actually > + * necessary/useful for shutdown. > + */ > + > + status = ssam_notifier_disable_registered(c); > + if (status) { > + ssam_err(c, "pm: failed to disable notifiers for shutdown: %d\n", > + status); > + } > + > + status = ssam_ctrl_notif_display_off(c); > + if (status) > + ssam_err(c, "pm: display-off notification failed: %d\n", status); > + > + status = ssam_ctrl_notif_d0_exit(c); > + if (status) > + ssam_err(c, "pm: D0-exit notification failed: %d\n", status); > +} > + > +#ifdef CONFIG_PM_SLEEP > + > +static int ssam_serial_hub_pm_prepare(struct device *dev) > +{ > + struct ssam_controller *c = dev_get_drvdata(dev); > + int status; > + > + /* > + * Try to signal display-off, This will quiesce events. > + * > + * Note: Signaling display-off/display-on should normally be done from > + * some sort of display state notifier. As that is not available, > + * signal it here. > + */ > + > + status = ssam_ctrl_notif_display_off(c); > + if (status) > + ssam_err(c, "pm: display-off notification failed: %d\n", status); > + > + return status; > +} > + > +static void ssam_serial_hub_pm_complete(struct device *dev) > +{ > + struct ssam_controller *c = dev_get_drvdata(dev); > + int status; > + > + /* > + * Try to signal display-on. This will restore events. > + * > + * Note: Signaling display-off/display-on should normally be done from > + * some sort of display state notifier. As that is not available, > + * signal it here. > + */ > + > + status = ssam_ctrl_notif_display_on(c); > + if (status) > + ssam_err(c, "pm: display-on notification failed: %d\n", status); > +} > + > +static int ssam_serial_hub_pm_suspend(struct device *dev) > +{ > + struct ssam_controller *c = dev_get_drvdata(dev); > + int status; > + > + /* > + * Try to signal D0-exit, enable IRQ wakeup if specified. Abort on > + * error. > + */ > + > + status = ssam_ctrl_notif_d0_exit(c); > + if (status) { > + ssam_err(c, "pm: D0-exit notification failed: %d\n", status); > + goto err_notif; > + } > + > + status = ssam_irq_arm_for_wakeup(c); > + if (status) > + goto err_irq; > + > + WARN_ON(ssam_controller_suspend(c)); > + return 0; > + > +err_irq: > + ssam_ctrl_notif_d0_entry(c); > +err_notif: > + ssam_ctrl_notif_display_on(c); > + return status; > +} > + > +static int ssam_serial_hub_pm_resume(struct device *dev) > +{ > + struct ssam_controller *c = dev_get_drvdata(dev); > + int status; > + > + WARN_ON(ssam_controller_resume(c)); > + > + /* > + * Try to disable IRQ wakeup (if specified) and signal D0-entry. In > + * case of errors, log them and try to restore normal operation state > + * as far as possible. > + * > + * Note: Signaling display-off/display-on should normally be done from > + * some sort of display state notifier. As that is not available, > + * signal it here. > + */ > + > + ssam_irq_disarm_wakeup(c); > + > + status = ssam_ctrl_notif_d0_entry(c); > + if (status) > + ssam_err(c, "pm: D0-entry notification failed: %d\n", status); > + > + return 0; > +} > + > +static int ssam_serial_hub_pm_freeze(struct device *dev) > +{ > + struct ssam_controller *c = dev_get_drvdata(dev); > + int status; > + > + /* > + * During hibernation image creation, we only have to ensure that the > + * EC doesn't send us any events. This is done via the display-off > + * and D0-exit notifications. Note that this sets up the wakeup IRQ > + * on the EC side, however, we have disabled it by default on our side > + * and won't enable it here. > + * > + * See ssam_serial_hub_poweroff() for more details on the hibernation > + * process. > + */ > + > + status = ssam_ctrl_notif_d0_exit(c); > + if (status) { > + ssam_err(c, "pm: D0-exit notification failed: %d\n", status); > + ssam_ctrl_notif_display_on(c); > + return status; > + } > + > + WARN_ON(ssam_controller_suspend(c)); > + return 0; > +} > + > +static int ssam_serial_hub_pm_thaw(struct device *dev) > +{ > + struct ssam_controller *c = dev_get_drvdata(dev); > + int status; > + > + WARN_ON(ssam_controller_resume(c)); > + > + status = ssam_ctrl_notif_d0_entry(c); > + if (status) > + ssam_err(c, "pm: D0-exit notification failed: %d\n", status); > + > + return status; > +} > + > +static int ssam_serial_hub_pm_poweroff(struct device *dev) > +{ > + struct ssam_controller *c = dev_get_drvdata(dev); > + int status; > + > + /* > + * When entering hibernation and powering off the system, the EC, at > + * least on some models, may disable events. Without us taking care of > + * that, this leads to events not being enabled/restored when the > + * system resumes from hibernation, resulting SAM-HID subsystem devices > + * (i.e. keyboard, touchpad) not working, AC-plug/AC-unplug events being > + * gone, etc. > + * > + * To avoid these issues, we disable all registered events here (this is > + * likely not actually required) and restore them during the drivers PM > + * restore callback. > + * > + * Wakeup from the EC interrupt is not supported during hibernation, > + * so don't arm the IRQ here. > + */ > + > + status = ssam_notifier_disable_registered(c); > + if (status) { > + ssam_err(c, "pm: failed to disable notifiers for hibernation: %d\n", > + status); > + return status; > + } > + > + status = ssam_ctrl_notif_d0_exit(c); > + if (status) { > + ssam_err(c, "pm: D0-exit notification failed: %d\n", status); > + ssam_notifier_restore_registered(c); > + return status; > + } > + > + WARN_ON(ssam_controller_suspend(c)); > + return 0; > +} > + > +static int ssam_serial_hub_pm_restore(struct device *dev) > +{ > + struct ssam_controller *c = dev_get_drvdata(dev); > + int status; > + > + /* > + * Ignore but log errors, try to restore state as much as possible in > + * case of failures. See ssam_serial_hub_poweroff() for more details on > + * the hibernation process. > + */ > + > + WARN_ON(ssam_controller_resume(c)); > + > + status = ssam_ctrl_notif_d0_entry(c); > + if (status) > + ssam_err(c, "pm: D0-entry notification failed: %d\n", status); > + > + ssam_notifier_restore_registered(c); > + return 0; > +} > + > +static const struct dev_pm_ops ssam_serial_hub_pm_ops = { > + .prepare = ssam_serial_hub_pm_prepare, > + .complete = ssam_serial_hub_pm_complete, > + .suspend = ssam_serial_hub_pm_suspend, > + .resume = ssam_serial_hub_pm_resume, > + .freeze = ssam_serial_hub_pm_freeze, > + .thaw = ssam_serial_hub_pm_thaw, > + .poweroff = ssam_serial_hub_pm_poweroff, > + .restore = ssam_serial_hub_pm_restore, > +}; > + > +#else /* CONFIG_PM_SLEEP */ > + > +static const struct dev_pm_ops ssam_serial_hub_pm_ops = { }; > + > +#endif /* CONFIG_PM_SLEEP */ > + > + > +/* -- Device/driver setup. -------------------------------------------------- */ > + > +static const struct acpi_gpio_params gpio_ssam_wakeup_int = { 0, 0, false }; > +static const struct acpi_gpio_params gpio_ssam_wakeup = { 1, 0, false }; > + > +static const struct acpi_gpio_mapping ssam_acpi_gpios[] = { > + { "ssam_wakeup-int-gpio", &gpio_ssam_wakeup_int, 1 }, > + { "ssam_wakeup-gpio", &gpio_ssam_wakeup, 1 }, > + { }, > +}; > + > +static int ssam_serial_hub_probe(struct serdev_device *serdev) > +{ > + struct ssam_controller *ctrl; > + acpi_handle *ssh = ACPI_HANDLE(&serdev->dev); > + acpi_status astatus; > + int status; > + > + if (gpiod_count(&serdev->dev, NULL) < 0) > + return -ENODEV; > + > + status = devm_acpi_dev_add_driver_gpios(&serdev->dev, ssam_acpi_gpios); > + if (status) > + return status; > + > + /* Allocate controller. */ > + ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL); > + if (!ctrl) > + return -ENOMEM; > + > + /* Initialize controller. */ > + status = ssam_controller_init(ctrl, serdev); > + if (status) > + goto err_ctrl_init; > + > + /* Set up serdev device. */ > + serdev_device_set_drvdata(serdev, ctrl); > + serdev_device_set_client_ops(serdev, &ssam_serdev_ops); > + status = serdev_device_open(serdev); > + if (status) > + goto err_devopen; > + > + astatus = ssam_serdev_setup_via_acpi(ssh, serdev); > + if (ACPI_FAILURE(astatus)) { > + status = -ENXIO; > + goto err_devinit; > + } > + > + /* Start controller. */ > + status = ssam_controller_start(ctrl); > + if (status) > + goto err_devinit; > + > + /* > + * Initial SAM requests: Log version and notify default/init power > + * states. > + */ > + status = ssam_log_firmware_version(ctrl); > + if (status) > + goto err_initrq; > + > + status = ssam_ctrl_notif_d0_entry(ctrl); > + if (status) > + goto err_initrq; > + > + status = ssam_ctrl_notif_display_on(ctrl); > + if (status) > + goto err_initrq; > + > + status = sysfs_create_group(&serdev->dev.kobj, &ssam_sam_group); > + if (status) > + goto err_initrq; > + > + /* Set up IRQ. */ > + status = ssam_irq_setup(ctrl); > + if (status) > + goto err_irq; > + > + /* Finally, set main controller reference. */ > + status = ssam_try_set_controller(ctrl); > + if (WARN_ON(status)) /* Currently, we're the only provider. */ > + goto err_mainref; > + > + /* > + * TODO: The EC can wake up the system via the associated GPIO interrupt > + * in multiple situations. One of which is the remaining battery > + * capacity falling below a certain threshold. Normally, we should > + * use the device_init_wakeup function, however, the EC also seems > + * to have other reasons for waking up the system and it seems > + * that Windows has additional checks whether the system should be > + * resumed. In short, this causes some spurious unwanted wake-ups. > + * For now let's thus default power/wakeup to false. > + */ > + device_set_wakeup_capable(&serdev->dev, true); > + acpi_walk_dep_device_list(ssh); > + > + return 0; > + > +err_mainref: > + ssam_irq_free(ctrl); > +err_irq: > + sysfs_remove_group(&serdev->dev.kobj, &ssam_sam_group); > +err_initrq: > + ssam_controller_shutdown(ctrl); > +err_devinit: > + serdev_device_close(serdev); > +err_devopen: > + ssam_controller_destroy(ctrl); > +err_ctrl_init: > + kfree(ctrl); > + return status; > +} > + > +static void ssam_serial_hub_remove(struct serdev_device *serdev) > +{ > + struct ssam_controller *ctrl = serdev_device_get_drvdata(serdev); > + int status; > + > + /* Clear static reference so that no one else can get a new one. */ > + ssam_clear_controller(); > + > + /* Disable and free IRQ. */ > + ssam_irq_free(ctrl); > + > + sysfs_remove_group(&serdev->dev.kobj, &ssam_sam_group); > + ssam_controller_lock(ctrl); > + > + /* Act as if suspending to silence events. */ > + status = ssam_ctrl_notif_display_off(ctrl); > + if (status) { > + dev_err(&serdev->dev, "display-off notification failed: %d\n", > + status); > + } > + > + status = ssam_ctrl_notif_d0_exit(ctrl); > + if (status) { > + dev_err(&serdev->dev, "D0-exit notification failed: %d\n", > + status); > + } > + > + /* Shut down controller and remove serdev device reference from it. */ > + ssam_controller_shutdown(ctrl); > + > + /* Shut down actual transport. */ > + serdev_device_wait_until_sent(serdev, 0); > + serdev_device_close(serdev); > + > + /* Drop our controller reference. */ > + ssam_controller_unlock(ctrl); > + ssam_controller_put(ctrl); > + > + device_set_wakeup_capable(&serdev->dev, false); > +} > + > +static const struct acpi_device_id ssam_serial_hub_match[] = { > + { "MSHW0084", 0 }, > + { }, > +}; > +MODULE_DEVICE_TABLE(acpi, ssam_serial_hub_match); > + > +static struct serdev_device_driver ssam_serial_hub = { > + .probe = ssam_serial_hub_probe, > + .remove = ssam_serial_hub_remove, > + .driver = { > + .name = "surface_serial_hub", > + .acpi_match_table = ssam_serial_hub_match, > + .pm = &ssam_serial_hub_pm_ops, > + .shutdown = ssam_serial_hub_shutdown, > + .probe_type = PROBE_PREFER_ASYNCHRONOUS, > + }, > +}; > +module_serdev_device_driver(ssam_serial_hub); > + > +MODULE_AUTHOR("Maximilian Luz <luzmaximilian@xxxxxxxxx>"); > +MODULE_DESCRIPTION("Subsystem and Surface Serial Hub driver for Surface System Aggregator Module"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/platform/surface/aggregator/ssh_msgb.h b/drivers/platform/surface/aggregator/ssh_msgb.h > new file mode 100644 > index 000000000000..1221f642dda1 > --- /dev/null > +++ b/drivers/platform/surface/aggregator/ssh_msgb.h > @@ -0,0 +1,205 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * SSH message builder functions. > + * > + * Copyright (C) 2019-2020 Maximilian Luz <luzmaximilian@xxxxxxxxx> > + */ > + > +#ifndef _SURFACE_AGGREGATOR_SSH_MSGB_H > +#define _SURFACE_AGGREGATOR_SSH_MSGB_H > + > +#include <asm/unaligned.h> > +#include <linux/types.h> > + > +#include <linux/surface_aggregator/controller.h> > +#include <linux/surface_aggregator/serial_hub.h> > + > +/** > + * struct msgbuf - Buffer struct to construct SSH messages. > + * @begin: Pointer to the beginning of the allocated buffer space. > + * @end: Pointer to the end (one past last element) of the allocated buffer > + * space. > + * @ptr: Pointer to the first free element in the buffer. > + */ > +struct msgbuf { > + u8 *begin; > + u8 *end; > + u8 *ptr; > +}; > + > +/** > + * msgb_init() - Initialize the given message buffer struct. > + * @msgb: The buffer struct to initialize > + * @ptr: Pointer to the underlying memory by which the buffer will be backed. > + * @cap: Size of the underlying memory. > + * > + * Initialize the given message buffer struct using the provided memory as > + * backing. > + */ > +static inline void msgb_init(struct msgbuf *msgb, u8 *ptr, size_t cap) > +{ > + msgb->begin = ptr; > + msgb->end = ptr + cap; > + msgb->ptr = ptr; > +} > + > +/** > + * msgb_bytes_used() - Return the current number of bytes used in the buffer. > + * @msgb: The message buffer. > + */ > +static inline size_t msgb_bytes_used(const struct msgbuf *msgb) > +{ > + return msgb->ptr - msgb->begin; > +} > + > +static inline void __msgb_push_u8(struct msgbuf *msgb, u8 value) > +{ > + *msgb->ptr = value; > + msgb->ptr += sizeof(u8); > +} > + > +static inline void __msgb_push_u16(struct msgbuf *msgb, u16 value) > +{ > + put_unaligned_le16(value, msgb->ptr); > + msgb->ptr += sizeof(u16); > +} > + > +/** > + * msgb_push_u16() - Push a u16 value to the buffer. > + * @msgb: The message buffer. > + * @value: The value to push to the buffer. > + */ > +static inline void msgb_push_u16(struct msgbuf *msgb, u16 value) > +{ > + if (WARN_ON(msgb->ptr + sizeof(u16) > msgb->end)) > + return; > + > + __msgb_push_u16(msgb, value); > +} > + > +/** > + * msgb_push_syn() - Push SSH SYN bytes to the buffer. > + * @msgb: The message buffer. > + */ > +static inline void msgb_push_syn(struct msgbuf *msgb) > +{ > + msgb_push_u16(msgb, SSH_MSG_SYN); > +} > + > +/** > + * msgb_push_buf() - Push raw data to the buffer. > + * @msgb: The message buffer. > + * @buf: The data to push to the buffer. > + * @len: The length of the data to push to the buffer. > + */ > +static inline void msgb_push_buf(struct msgbuf *msgb, const u8 *buf, size_t len) > +{ > + msgb->ptr = memcpy(msgb->ptr, buf, len) + len; > +} > + > +/** > + * msgb_push_crc() - Compute CRC and push it to the buffer. > + * @msgb: The message buffer. > + * @buf: The data for which the CRC should be computed. > + * @len: The length of the data for which the CRC should be computed. > + */ > +static inline void msgb_push_crc(struct msgbuf *msgb, const u8 *buf, size_t len) > +{ > + msgb_push_u16(msgb, ssh_crc(buf, len)); > +} > + > +/** > + * msgb_push_frame() - Push a SSH message frame header to the buffer. > + * @msgb: The message buffer > + * @ty: The type of the frame. > + * @len: The length of the payload of the frame. > + * @seq: The sequence ID of the frame/packet. > + */ > +static inline void msgb_push_frame(struct msgbuf *msgb, u8 ty, u16 len, u8 seq) > +{ > + u8 *const begin = msgb->ptr; > + > + if (WARN_ON(msgb->ptr + sizeof(struct ssh_frame) > msgb->end)) > + return; > + > + __msgb_push_u8(msgb, ty); /* Frame type. */ > + __msgb_push_u16(msgb, len); /* Frame payload length. */ > + __msgb_push_u8(msgb, seq); /* Frame sequence ID. */ > + > + msgb_push_crc(msgb, begin, msgb->ptr - begin); > +} > + > +/** > + * msgb_push_ack() - Push a SSH ACK frame to the buffer. > + * @msgb: The message buffer > + * @seq: The sequence ID of the frame/packet to be ACKed. > + */ > +static inline void msgb_push_ack(struct msgbuf *msgb, u8 seq) > +{ > + /* SYN. */ > + msgb_push_syn(msgb); > + > + /* ACK-type frame + CRC. */ > + msgb_push_frame(msgb, SSH_FRAME_TYPE_ACK, 0x00, seq); > + > + /* Payload CRC (ACK-type frames do not have a payload). */ > + msgb_push_crc(msgb, msgb->ptr, 0); > +} > + > +/** > + * msgb_push_nak() - Push a SSH NAK frame to the buffer. > + * @msgb: The message buffer > + */ > +static inline void msgb_push_nak(struct msgbuf *msgb) > +{ > + /* SYN. */ > + msgb_push_syn(msgb); > + > + /* NAK-type frame + CRC. */ > + msgb_push_frame(msgb, SSH_FRAME_TYPE_NAK, 0x00, 0x00); > + > + /* Payload CRC (ACK-type frames do not have a payload). */ > + msgb_push_crc(msgb, msgb->ptr, 0); > +} > + > +/** > + * msgb_push_cmd() - Push a SSH command frame with payload to the buffer. > + * @msgb: The message buffer. > + * @seq: The sequence ID (SEQ) of the frame/packet. > + * @rqid: The request ID (RQID) of the request contained in the frame. > + * @rqst: The request to wrap in the frame. > + */ > +static inline void msgb_push_cmd(struct msgbuf *msgb, u8 seq, u16 rqid, > + const struct ssam_request *rqst) > +{ > + const u8 type = SSH_FRAME_TYPE_DATA_SEQ; > + u8 *cmd; > + > + /* SYN. */ > + msgb_push_syn(msgb); > + > + /* Command frame + CRC. */ > + msgb_push_frame(msgb, type, sizeof(struct ssh_command) + rqst->length, seq); > + > + /* Frame payload: Command struct + payload. */ > + if (WARN_ON(msgb->ptr + sizeof(struct ssh_command) > msgb->end)) > + return; > + > + cmd = msgb->ptr; > + > + __msgb_push_u8(msgb, SSH_PLD_TYPE_CMD); /* Payload type. */ > + __msgb_push_u8(msgb, rqst->target_category); /* Target category. */ > + __msgb_push_u8(msgb, rqst->target_id); /* Target ID (out). */ > + __msgb_push_u8(msgb, 0x00); /* Target ID (in). */ > + __msgb_push_u8(msgb, rqst->instance_id); /* Instance ID. */ > + __msgb_push_u16(msgb, rqid); /* Request ID. */ > + __msgb_push_u8(msgb, rqst->command_id); /* Command ID. */ > + > + /* Command payload. */ > + msgb_push_buf(msgb, rqst->payload, rqst->length); > + > + /* CRC for command struct + payload. */ > + msgb_push_crc(msgb, cmd, msgb->ptr - cmd); > +} > + > +#endif /* _SURFACE_AGGREGATOR_SSH_MSGB_H */ > diff --git a/drivers/platform/surface/aggregator/ssh_packet_layer.c b/drivers/platform/surface/aggregator/ssh_packet_layer.c > new file mode 100644 > index 000000000000..237d28c90e4b > --- /dev/null > +++ b/drivers/platform/surface/aggregator/ssh_packet_layer.c > @@ -0,0 +1,1699 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * SSH packet transport layer. > + * > + * Copyright (C) 2019-2020 Maximilian Luz <luzmaximilian@xxxxxxxxx> > + */ > + > +#include <asm/unaligned.h> > +#include <linux/atomic.h> > +#include <linux/jiffies.h> > +#include <linux/kfifo.h> > +#include <linux/kref.h> > +#include <linux/kthread.h> > +#include <linux/ktime.h> > +#include <linux/limits.h> > +#include <linux/list.h> > +#include <linux/serdev.h> > +#include <linux/slab.h> > +#include <linux/spinlock.h> > +#include <linux/workqueue.h> > + > +#include <linux/surface_aggregator/serial_hub.h> > + > +#include "ssh_msgb.h" > +#include "ssh_packet_layer.h" > +#include "ssh_parser.h" > + > +/* > + * To simplify reasoning about the code below, we define a few concepts. The > + * system below is similar to a state-machine for packets, however, there are > + * too many states to explicitly write them down. To (somewhat) manage the > + * states and packets we rely on flags, reference counting, and some simple > + * concepts. State transitions are triggered by actions. > + * > + * >> Actions << > + * > + * - submit > + * - transmission start (process next item in queue) > + * - transmission finished (guaranteed to never be parallel to transmission > + * start) > + * - ACK received > + * - NAK received (this is equivalent to issuing re-submit for all pending > + * packets) > + * - timeout (this is equivalent to re-issuing a submit or canceling) > + * - cancel (non-pending and pending) > + * > + * >> Data Structures, Packet Ownership, General Overview << > + * > + * The code below employs two main data structures: The packet queue, > + * containing all packets scheduled for transmission, and the set of pending > + * packets, containing all packets awaiting an ACK. > + * > + * Shared ownership of a packet is controlled via reference counting. Inside > + * the transport system are a total of five packet owners: > + * > + * - the packet queue, > + * - the pending set, > + * - the transmitter thread, > + * - the receiver thread (via ACKing), and > + * - the timeout work item. > + * > + * Normal operation is as follows: The initial reference of the packet is > + * obtained by submitting the packet and queuing it. The receiver thread takes > + * packets from the queue. By doing this, it does not increment the refcount > + * but takes over the reference (removing it from the queue). If the packet is > + * sequenced (i.e. needs to be ACKed by the client), the transmitter thread > + * sets-up the timeout and adds the packet to the pending set before starting > + * to transmit it. As the timeout is handled by a reaper task, no additional > + * reference for it is needed. After the transmit is done, the reference held > + * by the transmitter thread is dropped. If the packet is unsequenced (i.e. > + * does not need an ACK), the packet is completed by the transmitter thread > + * before dropping that reference. > + * > + * On receival of an ACK, the receiver thread removes and obtains the > + * reference to the packet from the pending set. The receiver thread will then > + * complete the packet and drop its reference. > + * > + * On receival of a NAK, the receiver thread re-submits all currently pending > + * packets. > + * > + * Packet timeouts are detected by the timeout reaper. This is a task, > + * scheduled depending on the earliest packet timeout expiration date, > + * checking all currently pending packets if their timeout has expired. If the > + * timeout of a packet has expired, it is re-submitted and the number of tries > + * of this packet is incremented. If this number reaches its limit, the packet > + * will be completed with a failure. > + * > + * On transmission failure (such as repeated packet timeouts), the completion > + * callback is immediately run by on thread on which the error was detected. > + * > + * To ensure that a packet eventually leaves the system it is marked as > + * "locked" directly before it is going to be completed or when it is > + * canceled. Marking a packet as "locked" has the effect that passing and > + * creating new references of the packet is disallowed. This means that the > + * packet cannot be added to the queue, the pending set, and the timeout, or > + * be picked up by the transmitter thread or receiver thread. To remove a > + * packet from the system it has to be marked as locked and subsequently all > + * references from the data structures (queue, pending) have to be removed. > + * References held by threads will eventually be dropped automatically as > + * their execution progresses. > + * > + * Note that the packet completion callback is, in case of success and for a > + * sequenced packet, guaranteed to run on the receiver thread, thus providing > + * a way to reliably identify responses to the packet. The packet completion > + * callback is only run once and it does not indicate that the packet has > + * fully left the system (for this, one should rely on the release method, > + * triggered when the reference count of the packet reaches zero). In case of > + * re-submission (and with somewhat unlikely timing), it may be possible that > + * the packet is being re-transmitted while the completion callback runs. > + * Completion will occur both on success and internal error, as well as when > + * the packet is canceled. > + * > + * >> Flags << > + * > + * Flags are used to indicate the state and progression of a packet. Some flags > + * have stricter guarantees than other: > + * > + * - locked > + * Indicates if the packet is locked. If the packet is locked, passing and/or > + * creating additional references to the packet is forbidden. The packet thus > + * may not be queued, dequeued, or removed or added to the pending set. Note > + * that the packet state flags may still change (e.g. it may be marked as > + * ACKed, transmitted, ...). > + * > + * - completed > + * Indicates if the packet completion callback has been executed or is about > + * to be executed. This flag is used to ensure that the packet completion > + * callback is only run once. > + * > + * - queued > + * Indicates if a packet is present in the submission queue or not. This flag > + * must only be modified with the queue lock held, and must be coherent to the > + * presence of the packet in the queue. > + * > + * - pending > + * Indicates if a packet is present in the set of pending packets or not. > + * This flag must only be modified with the pending lock held, and must be > + * coherent to the presence of the packet in the pending set. > + * > + * - transmitting > + * Indicates if the packet is currently transmitting. In case of > + * re-transmissions, it is only safe to wait on the "transmitted" completion > + * after this flag has been set. The completion will be set both in success > + * and error case. > + * > + * - transmitted > + * Indicates if the packet has been transmitted. This flag is not cleared by > + * the system, thus it indicates the first transmission only. > + * > + * - acked > + * Indicates if the packet has been acknowledged by the client. There are no > + * other guarantees given. For example, the packet may still be canceled > + * and/or the completion may be triggered an error even though this bit is > + * set. Rely on the status provided to the completion callback instead. > + * > + * - canceled > + * Indicates if the packet has been canceled from the outside. There are no > + * other guarantees given. Specifically, the packet may be completed by > + * another part of the system before the cancellation attempts to complete it. > + * > + * >> General Notes << > + * > + * - To avoid deadlocks, if both queue and pending locks are required, the > + * pending lock must be acquired before the queue lock. > + * > + * - The packet priority must be accessed only while holding the queue lock. > + * > + * - The packet timestamp must be accessed only while holding the pending > + * lock. > + */ > + > +/* > + * SSH_PTL_MAX_PACKET_TRIES - Maximum transmission attempts for packet. > + * > + * Maximum number of transmission attempts per sequenced packet in case of > + * time-outs. Must be smaller than 16. If the packet times out after this > + * amount of tries, the packet will be completed with %-ETIMEDOUT as status > + * code. > + */ > +#define SSH_PTL_MAX_PACKET_TRIES 3 > + > +/* > + * SSH_PTL_TX_TIMEOUT - Packet transmission timeout. > + * > + * Timeout in jiffies for packet transmission via the underlying serial > + * device. If transmitting the packet takes longer than this timeout, the > + * packet will be completed with -ETIMEDOUT. It will not be re-submitted. > + */ > +#define SSH_PTL_TX_TIMEOUT HZ > + > +/* > + * SSH_PTL_PACKET_TIMEOUT - Packet response timeout. > + * > + * Timeout as ktime_t delta for ACKs. If we have not received an ACK in this > + * time-frame after starting transmission, the packet will be re-submitted. > + */ > +#define SSH_PTL_PACKET_TIMEOUT ms_to_ktime(1000) > + > +/* > + * SSH_PTL_PACKET_TIMEOUT_RESOLUTION - Packet timeout granularity. > + * > + * Time-resolution for timeouts. Should be larger than one jiffy to avoid > + * direct re-scheduling of reaper work_struct. > + */ > +#define SSH_PTL_PACKET_TIMEOUT_RESOLUTION ms_to_ktime(max(2000 / HZ, 50)) > + > +/* > + * SSH_PTL_MAX_PENDING - Maximum number of pending packets. > + * > + * Maximum number of sequenced packets concurrently waiting for an ACK. > + * Packets marked as blocking will not be transmitted while this limit is > + * reached. > + */ > +#define SSH_PTL_MAX_PENDING 1 > + > +/* > + * SSH_PTL_RX_BUF_LEN - Evaluation-buffer size in bytes. > + */ > +#define SSH_PTL_RX_BUF_LEN 4096 > + > +/* > + * SSH_PTL_RX_FIFO_LEN - Fifo input-buffer size in bytes. > + */ > +#define SSH_PTL_RX_FIFO_LEN 4096 > + > +static void __ssh_ptl_packet_release(struct kref *kref) > +{ > + struct ssh_packet *p = container_of(kref, struct ssh_packet, refcnt); > + > + ptl_dbg_cond(p->ptl, "ptl: releasing packet %p\n", p); > + p->ops->release(p); > +} > + > +/** > + * ssh_packet_get() - Increment reference count of packet. > + * @packet: The packet to increment the reference count of. > + * > + * Increments the reference count of the given packet. See ssh_packet_put() > + * for the counter-part of this function. > + * > + * Return: Returns the packet provided as input. > + */ > +struct ssh_packet *ssh_packet_get(struct ssh_packet *packet) > +{ > + if (packet) > + kref_get(&packet->refcnt); > + return packet; > +} > +EXPORT_SYMBOL_GPL(ssh_packet_get); > + > +/** > + * ssh_packet_put() - Decrement reference count of packet. > + * @packet: The packet to decrement the reference count of. > + * > + * If the reference count reaches zero, the ``release`` callback specified in > + * the packet's &struct ssh_packet_ops, i.e. ``packet->ops->release``, will be > + * called. > + * > + * See ssh_packet_get() for the counter-part of this function. > + */ > +void ssh_packet_put(struct ssh_packet *packet) > +{ > + if (packet) > + kref_put(&packet->refcnt, __ssh_ptl_packet_release); > +} > +EXPORT_SYMBOL_GPL(ssh_packet_put); > + > +static u8 ssh_packet_get_seq(struct ssh_packet *packet) > +{ > + return packet->data.ptr[SSH_MSGOFFSET_FRAME(seq)]; > +} > + > +/** > + * ssh_packet_init() - Initialize SSH packet. > + * @packet: The packet to initialize. > + * @type: Type-flags of the packet. > + * @priority: Priority of the packet. See SSH_PACKET_PRIORITY() for details. > + * @ops: Packet operations. > + * > + * Initializes the given SSH packet. Sets the transmission buffer pointer to > + * %NULL and the transmission buffer length to zero. For data-type packets, > + * this buffer has to be set separately via ssh_packet_set_data() before > + * submission, and must contain a valid SSH message, i.e. frame with optional > + * payload of any type. > + */ > +void ssh_packet_init(struct ssh_packet *packet, unsigned long type, > + u8 priority, const struct ssh_packet_ops *ops) > +{ > + kref_init(&packet->refcnt); > + > + packet->ptl = NULL; > + INIT_LIST_HEAD(&packet->queue_node); > + INIT_LIST_HEAD(&packet->pending_node); > + > + packet->state = type & SSH_PACKET_FLAGS_TY_MASK; > + packet->priority = priority; > + packet->timestamp = KTIME_MAX; > + > + packet->data.ptr = NULL; > + packet->data.len = 0; > + > + packet->ops = ops; > +} > + > +/** > + * ssh_ctrl_packet_alloc() - Allocate control packet. > + * @packet: Where the pointer to the newly allocated packet should be stored. > + * @buffer: The buffer corresponding to this packet. > + * @flags: Flags used for allocation. > + * > + * Allocates a packet and corresponding transport buffer. Sets the packet's > + * buffer reference to the allocated buffer. The packet must be freed via > + * ssh_ctrl_packet_free(), which will also free the corresponding buffer. The > + * corresponding buffer must not be freed separately. Intended to be used with > + * %ssh_ptl_ctrl_packet_ops as packet operations. > + * > + * Return: Returns zero on success, %-ENOMEM if the allocation failed. > + */ > +static int ssh_ctrl_packet_alloc(struct ssh_packet **packet, > + struct ssam_span *buffer, gfp_t flags) > +{ > + *packet = kzalloc(sizeof(**packet) + SSH_MSG_LEN_CTRL, flags); > + if (!*packet) > + return -ENOMEM; > + > + buffer->ptr = (u8 *)(*packet + 1); > + buffer->len = SSH_MSG_LEN_CTRL; > + > + return 0; > +} > + > +/** > + * ssh_ctrl_packet_free() - Free control packet. > + * @p: The packet to free. > + */ > +static void ssh_ctrl_packet_free(struct ssh_packet *p) > +{ > + kfree(p); > +} > + > +static const struct ssh_packet_ops ssh_ptl_ctrl_packet_ops = { > + .complete = NULL, > + .release = ssh_ctrl_packet_free, > +}; > + > +static void ssh_ptl_timeout_reaper_mod(struct ssh_ptl *ptl, ktime_t now, > + ktime_t expires) > +{ > + unsigned long delta = msecs_to_jiffies(ktime_ms_delta(expires, now)); > + ktime_t aexp = ktime_add(expires, SSH_PTL_PACKET_TIMEOUT_RESOLUTION); > + > + spin_lock(&ptl->rtx_timeout.lock); > + > + /* Re-adjust / schedule reaper only if it is above resolution delta. */ > + if (ktime_before(aexp, ptl->rtx_timeout.expires)) { > + ptl->rtx_timeout.expires = expires; > + mod_delayed_work(system_wq, &ptl->rtx_timeout.reaper, delta); > + } > + > + spin_unlock(&ptl->rtx_timeout.lock); > +} > + > +/* Must be called with queue lock held. */ > +static void ssh_packet_next_try(struct ssh_packet *p) > +{ > + u8 base = ssh_packet_priority_get_base(p->priority); > + u8 try = ssh_packet_priority_get_try(p->priority); > + > + p->priority = __SSH_PACKET_PRIORITY(base, try + 1); > +} > + > +/* Must be called with queue lock held. */ > +static struct list_head *__ssh_ptl_queue_find_entrypoint(struct ssh_packet *p) > +{ > + struct list_head *head; > + struct ssh_packet *q; > + > + /* > + * We generally assume that there are less control (ACK/NAK) packets > + * and re-submitted data packets as there are normal data packets (at > + * least in situations in which many packets are queued; if there > + * aren't many packets queued the decision on how to iterate should be > + * basically irrelevant; the number of control/data packets is more or > + * less limited via the maximum number of pending packets). Thus, when > + * inserting a control or re-submitted data packet, (determined by > + * their priority), we search from front to back. Normal data packets > + * are, usually queued directly at the tail of the queue, so for those > + * search from back to front. > + */ > + > + if (p->priority > SSH_PACKET_PRIORITY(DATA, 0)) { > + list_for_each(head, &p->ptl->queue.head) { > + q = list_entry(head, struct ssh_packet, queue_node); > + > + if (q->priority < p->priority) > + break; > + } > + } else { > + list_for_each_prev(head, &p->ptl->queue.head) { > + q = list_entry(head, struct ssh_packet, queue_node); > + > + if (q->priority >= p->priority) { > + head = head->next; > + break; > + } > + } > + } > + > + return head; > +} > + > +/* Must be called with queue lock held. */ Maybe add a lockdep_assert for this ? (and the same for a bunch of similar cases below) > +static int __ssh_ptl_queue_push(struct ssh_packet *packet) > +{ > + struct ssh_ptl *ptl = packet->ptl; > + struct list_head *head; > + > + if (test_bit(SSH_PTL_SF_SHUTDOWN_BIT, &ptl->state)) > + return -ESHUTDOWN; > + > + /* Avoid further transitions when canceling/completing. */ > + if (test_bit(SSH_PACKET_SF_LOCKED_BIT, &packet->state)) > + return -EINVAL; > + > + /* If this packet has already been queued, do not add it. */ > + if (test_and_set_bit(SSH_PACKET_SF_QUEUED_BIT, &packet->state)) > + return -EALREADY; > + > + head = __ssh_ptl_queue_find_entrypoint(packet); > + > + list_add_tail(&ssh_packet_get(packet)->queue_node, head); > + return 0; > +} <snip about 5000 more lines> Snipped because nothing stood out in the rest of this patch. Phew that was one large patch to review. I've run out of review bandwidth for today, so I will review the rest of the series later (probably coming Thursday). Regards, Hans
