delta/linux.git/drivers/platform/surface/aggregator, branch master

platform/surface: aggregator: Add missing fwnode_handle_put()

2023-03-22T14:23:03+00:00

In fwnode_for_each_child_node(), we should add
fwnode_handle_put() when break out of the iteration
fwnode_for_each_child_node() as it will automatically
increase and decrease the refcounter.

Fixes: fc622b3d36e6 ("platform/surface: Set up Surface Aggregator device registry")
Signed-off-by: Liang He 
Reviewed-by: Maximilian Luz 
Link: https://lore.kernel.org/r/20230322033057.1855741-1-windhl@126.com
Reviewed-by: Hans de Goede 
Signed-off-by: Hans de Goede

Merge tag 'driver-core-6.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core

2023-02-24T20:58:55+00:00

Pull driver core updates from Greg KH:
 "Here is the large set of driver core changes for 6.3-rc1.

  There's a lot of changes this development cycle, most of the work
  falls into two different categories:

   - fw_devlink fixes and updates. This has gone through numerous review
     cycles and lots of review and testing by lots of different devices.
     Hopefully all should be good now, and Saravana will be keeping a
     watch for any potential regression on odd embedded systems.

   - driver core changes to work to make struct bus_type able to be
     moved into read-only memory (i.e. const) The recent work with Rust
     has pointed out a number of areas in the driver core where we are
     passing around and working with structures that really do not have
     to be dynamic at all, and they should be able to be read-only
     making things safer overall. This is the contuation of that work
     (started last release with kobject changes) in moving struct
     bus_type to be constant. We didn't quite make it for this release,
     but the remaining patches will be finished up for the release after
     this one, but the groundwork has been laid for this effort.

  Other than that we have in here:

   - debugfs memory leak fixes in some subsystems

   - error path cleanups and fixes for some never-able-to-be-hit
     codepaths.

   - cacheinfo rework and fixes

   - Other tiny fixes, full details are in the shortlog

  All of these have been in linux-next for a while with no reported
  problems"

[ Geert Uytterhoeven points out that that last sentence isn't true, and
  that there's a pending report that has a fix that is queued up - Linus ]

* tag 'driver-core-6.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core: (124 commits)
  debugfs: drop inline constant formatting for ERR_PTR(-ERROR)
  OPP: fix error checking in opp_migrate_dentry()
  debugfs: update comment of debugfs_rename()
  i3c: fix device.h kernel-doc warnings
  dma-mapping: no need to pass a bus_type into get_arch_dma_ops()
  driver core: class: move EXPORT_SYMBOL_GPL() lines to the correct place
  Revert "driver core: add error handling for devtmpfs_create_node()"
  Revert "devtmpfs: add debug info to handle()"
  Revert "devtmpfs: remove return value of devtmpfs_delete_node()"
  driver core: cpu: don't hand-override the uevent bus_type callback.
  devtmpfs: remove return value of devtmpfs_delete_node()
  devtmpfs: add debug info to handle()
  driver core: add error handling for devtmpfs_create_node()
  driver core: bus: update my copyright notice
  driver core: bus: add bus_get_dev_root() function
  driver core: bus: constify bus_unregister()
  driver core: bus: constify some internal functions
  driver core: bus: constify bus_get_kset()
  driver core: bus: constify bus_register/unregister_notifier()
  driver core: remove private pointer from struct bus_type
  ...

platform/surface: aggregator: Rename top-level request functions to avoid ambiguities

2023-02-02T21:48:20+00:00

We currently have a struct ssam_request_sync and a function
ssam_request_sync(). While this is valid C, there are some downsides to
it.

One of these is that current Sphinx versions (>= 3.0) cannot
disambiguate between the two (see disucssion and pull request linked
below). It instead emits a "WARNING: Duplicate C declaration" and links
for the struct and function in the resulting documentation link to the
same entry (i.e. both to either function or struct documentation)
instead of their respective own entries.

While we could just ignore that and wait for a fix, there's also a point
to be made that the current naming can be somewhat confusing when
searching (e.g. via grep) or trying to understand the levels of
abstraction at play:

We currently have struct ssam_request_sync and associated functions
ssam_request_sync_[alloc|free|init|wait|...]() operating on this struct.
However, function ssam_request_sync() is one abstraction level above
this. Similarly, ssam_request_sync_with_buffer() is not a function
operating on struct ssam_request_sync, but rather a sibling to
ssam_request_sync(), both using the struct under the hood.

Therefore, rename the top level request functions:

  ssam_request_sync() -> ssam_request_do_sync()
  ssam_request_sync_with_buffer() -> ssam_request_do_sync_with_buffer()
  ssam_request_sync_onstack() -> ssam_request_do_sync_onstack()

Link: https://lore.kernel.org/all/085e0ada65c11da9303d07e70c510dc45f21315b.1656756450.git.mchehab@kernel.org/
Link: https://github.com/sphinx-doc/sphinx/pull/8313
Signed-off-by: Maximilian Luz 
Link: https://lore.kernel.org/r/20221220175608.1436273-2-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede 
Signed-off-by: Hans de Goede

platform/surface: aggregator: Add target and source IDs to command trace events

2023-02-02T21:48:20+00:00

Add command source and target IDs to trace events.

Tracing support for the Surface Aggregator driver was originally
implemented at a time when only two peers were known: Host and SAM. We
now know that there are at least five, with three actively being used
(Host, SAM, KIP; four with Debug if you want to count manually enabling
that interface). So it makes sense to also explicitly name the peers
involved when tracing.

Signed-off-by: Maximilian Luz 
Link: https://lore.kernel.org/r/20221202223327.690880-4-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede 
Signed-off-by: Hans de Goede

platform/surface: aggregator: Improve documentation and handling of message target and source IDs

2023-02-02T21:48:20+00:00

The `tid_in` and `tid_out` fields of the serial hub protocol command
struct (struct ssh_command) are actually source and target IDs,
indicating the peer from which the message originated and the peer for
which it is intended.

Change the naming of those fields accordingly and improve the protocol
documentation. Additionally, introduce an enum containing all currently
known peers, i.e. targets and sources.

Signed-off-by: Maximilian Luz 
Link: https://lore.kernel.org/r/20221202223327.690880-3-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede 
Signed-off-by: Hans de Goede

driver core: make struct device_type.uevent() take a const *

2023-01-27T12:45:36+00:00

The uevent() callback in struct device_type should not be modifying the
device that is passed into it, so mark it as a const * and propagate the
function signature changes out into all relevant subsystems that use
this callback.

Cc: Alan Stern 
Cc: Andreas Noever 
Cc: Andy Shevchenko 
Cc: Bard Liao 
Cc: Chaitanya Kulkarni 
Cc: Dan Williams 
Cc: Dmitry Torokhov 
Cc: Frank Rowand 
Cc: Ira Weiny 
Cc: Jason Gunthorpe 
Cc: Jens Axboe 
Cc: Jilin Yuan 
Cc: Jiri Slaby 
Cc: Len Brown 
Cc: Mark Gross 
Cc: "Martin K. Petersen" 
Cc: "Matthew Wilcox (Oracle)" 
Cc: Maximilian Luz 
Cc: Michael Jamet 
Cc: Ming Lei 
Cc: Pierre-Louis Bossart 
Cc: Rob Herring 
Cc: Sakari Ailus 
Cc: Sanyog Kale 
Cc: Sean Young 
Cc: Stefan Richter 
Cc: Thomas Gleixner 
Cc: Won Chung 
Cc: Yehezkel Bernat 
Acked-by: Rafael J. Wysocki 
Acked-by: Mika Westerberg  # for Thunderbolt
Acked-by: Mauro Carvalho Chehab 
Acked-by: Alexandre Belloni 
Acked-by: Heikki Krogerus 
Acked-by: Wolfram Sang 
Acked-by: Vinod Koul 
Acked-by: Hans de Goede 
Link: https://lore.kernel.org/r/20230111113018.459199-6-gregkh@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman

platform/surface: aggregator: Add missing call to ssam_request_sync_free()

2023-01-12T18:12:02+00:00

Although rare, ssam_request_sync_init() can fail. In that case, the
request should be freed via ssam_request_sync_free(). Currently it is
leaked instead. Fix this.

Fixes: c167b9c7e3d6 ("platform/surface: Add Surface Aggregator subsystem")
Signed-off-by: Maximilian Luz 
Link: https://lore.kernel.org/r/20221220175608.1436273-1-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede 
Signed-off-by: Hans de Goede

platform/surface: aggregator: Ignore command messages not intended for us

2023-01-12T18:11:52+00:00

It is possible that we (the host/kernel driver) receive command messages
that are not intended for us. Ignore those for now.

The whole story is a bit more complicated: It is possible to enable
debug output on SAM, which is sent via SSH command messages. By default
this output is sent to a debug connector, with its own target ID
(TID=0x03). It is possible to override the target of the debug output
and set it to the host/kernel driver. This, however, does not change the
original target ID of the message. Meaning, we receive messages with
TID=0x03 (debug) but expect to only receive messages with TID=0x00
(host).

The problem is that the different target ID also comes with a different
scope of request IDs. In particular, these do not follow the standard
event rules (i.e. do not fall into a set of small reserved values).
Therefore, current message handling interprets them as responses to
pending requests and tries to match them up via the request ID. However,
these debug output messages are not in fact responses, and therefore
this will at best fail to find the request and at worst pass on the
wrong data as response for a request.

Therefore ignore any command messages not intended for us (host) for
now. We can implement support for the debug messages once we have a
better understanding of them.

Note that this may also provide a bit more stability and avoid some
driver confusion in case any other targets want to talk to us in the
future, since we don't yet know what to do with those as well. A warning
for the dropped messages should suffice for now and also give us a
chance of discovering new targets if they come along without any
potential for bugs/instabilities.

Fixes: c167b9c7e3d6 ("platform/surface: Add Surface Aggregator subsystem")
Signed-off-by: Maximilian Luz 
Link: https://lore.kernel.org/r/20221202223327.690880-2-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede 
Signed-off-by: Hans de Goede

platform/surface: aggregator: Do not check for repeated unsequenced packets

2022-11-15T16:18:26+00:00

Currently, we check any received packet whether we have already seen it
previously, regardless of the packet type (sequenced / unsequenced). We
do this by checking the sequence number. This assumes that sequence
numbers are valid for both sequenced and unsequenced packets. However,
this assumption appears to be incorrect.

On some devices, the sequence number field of unsequenced packets (in
particular HID input events on the Surface Pro 9) is always zero. As a
result, the current retransmission check kicks in and discards all but
the first unsequenced packet, breaking (among other things) keyboard and
touchpad input.

Note that we have, so far, only seen packets being retransmitted in
sequenced communication. In particular, this happens when there is an
ACK timeout, causing the EC (or us) to re-send the packet waiting for an
ACK. Arguably, retransmission / duplication of unsequenced packets
should not be an issue as there is no logical condition (such as an ACK
timeout) to determine when a packet should be sent again.

Therefore, remove the retransmission check for unsequenced packets
entirely to resolve the issue.

Fixes: c167b9c7e3d6 ("platform/surface: Add Surface Aggregator subsystem")
Signed-off-by: Maximilian Luz 
Link: https://lore.kernel.org/r/20221113185951.224759-1-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede 
Signed-off-by: Hans de Goede

platform/surface: Update copyright year of various drivers

2022-07-02T09:23:30+00:00

Update the copyright of various Surface drivers to the current year.

Signed-off-by: Maximilian Luz 
Link: https://lore.kernel.org/r/20220624205800.1355621-4-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede 
Signed-off-by: Hans de Goede