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
...
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 <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20221220175608.1436273-2-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
The target ID of the base hub is currently set to KIP (keyboard/
peripherals). However, even though it manages such devices with the KIP
target ID, the base hub itself is actually accessed via the SAM target
ID. So set it accordingly.
Note that the target ID of the hub can be chosen arbitrarily and does
not directly correspond to any physical or virtual component of the EC.
This change is only a code improvement intended for consistency and
clarity, it does not fix an actual bug.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20221202223327.690880-10-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Similar to the target category (TC), the target ID (TID) can be one
value out of a small number of choices, given in enum ssam_ssh_tid.
In the device ID macros, SSAM_SDEV() and SSAM_VDEV() we already use text
expansion to, both, remove some textual clutter for the target category
values and enforce that the value belongs to the known set. Now that we
know the names for the target IDs, use the same trick for them as well.
Also rename the SSAM_ANY_x macros to SSAM_SSH_x_ANY to better fit in.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Link: https://lore.kernel.org/r/20221202223327.690880-9-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
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 <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20221202223327.690880-4-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
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 <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20221202223327.690880-3-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
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: c167b9c7e3 ("platform/surface: Add Surface Aggregator subsystem")
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20221202223327.690880-2-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Merge ACPI changes related to device enumeration, device object
managenet, operation region handling, table parsing and sysfs
interface:
- Use ZERO_PAGE(0) instead of empty_zero_page in the ACPI device
enumeration code (Giulio Benetti).
- Change the return type of the ACPI driver remove callback to void and
update its users accordingly (Dawei Li).
- Add general support for FFH address space type and implement the low-
level part of it for ARM64 (Sudeep Holla).
- Fix stale comments in the ACPI tables parsing code and make it print
more messages related to MADT (Hanjun Guo, Huacai Chen).
- Replace invocations of generic library functions with more kernel-
specific counterparts in the ACPI sysfs interface (Christophe JAILLET,
Xu Panda).
* acpi-scan:
ACPI: scan: substitute empty_zero_page with helper ZERO_PAGE(0)
* acpi-bus:
ACPI: FFH: Silence missing prototype warnings
ACPI: make remove callback of ACPI driver void
ACPI: bus: Fix the _OSC capability check for FFH OpRegion
arm64: Add architecture specific ACPI FFH Opregion callbacks
ACPI: Implement a generic FFH Opregion handler
* acpi-tables:
ACPI: tables: Fix the stale comments for acpi_locate_initial_tables()
ACPI: tables: Print CORE_PIC information when MADT is parsed
* acpi-sysfs:
ACPI: sysfs: use sysfs_emit() to instead of scnprintf()
ACPI: sysfs: Use kstrtobool() instead of strtobool()
For bus-based driver, device removal is implemented as:
1 device_remove()->
2 bus->remove()->
3 driver->remove()
Driver core needs no inform from callee(bus driver) about the
result of remove callback. In that case, commit fc7a6209d5
("bus: Make remove callback return void") forces bus_type::remove
be void-returned.
Now we have the situation that both 1 & 2 of calling chain are
void-returned, so it does not make much sense for 3(driver->remove)
to return non-void to its caller.
So the basic idea behind this change is making remove() callback of
any bus-based driver to be void-returned.
This change, for itself, is for device drivers based on acpi-bus.
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Lee Jones <lee@kernel.org>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Dawei Li <set_pte_at@outlook.com>
Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com> # for drivers/platform/surface/*
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Add device nodes to enable support for battery and charger status, the
ACPI platform profile, as well as internal and type-cover HID devices
(including sensors, touchpad, keyboard, and other miscellaneous devices)
on the Surface Pro 9.
This does not include support for a tablet-mode switch yet, as that is
now handled via the POS subsystem (unlike the Surface Pro 8, where it is
handled via the KIP subsystem) and therefore needs further changes.
While we're at it, also add the missing comment for the Surface Pro 8.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20221113185951.224759-2-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
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: c167b9c7e3 ("platform/surface: Add Surface Aggregator subsystem")
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20221113185951.224759-1-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Pull x86 platform driver updates from Hans de Goede:
- AMD Platform Management Framework (PMF) driver with AMT and QnQF
support
- AMD PMC: Improved logging for debugging s2idle issues
- Big refactor of the ACPI/x86 backlight handling, ensuring that we
only register 1 /sys/class/backlight device per LCD panel
- Microsoft Surface:
- Surface Laptop Go 2 support
- Surface Pro 8 HID sensor support
- Asus WMI:
- Lots of cleanups
- Support for TUF RGB keyboard backlight control
- Add support for ROG X13 tablet mode
- Siemens Simatic: IPC227G and IPC427G support
- Toshiba ACPI laptop driver: Fan hwmon and battery ECO mode support
- tools/power/x86/intel-speed-select: Various improvements
- Various cleanups
- Various small bugfixes
* tag 'platform-drivers-x86-v6.1-1' of git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86: (153 commits)
platform/x86: use PLATFORM_DEVID_NONE instead of -1
platform/x86/amd: pmc: Dump idle mask during "check" stage instead
platform/x86/intel/wmi: thunderbolt: Use dev_groups callback
platform/x86/amd: pmc: remove CONFIG_DEBUG_FS checks
platform/surface: Split memcpy() of struct ssam_event flexible array
platform/x86: compal-laptop: Get rid of a few forward declarations
platform/x86: intel-uncore-freq: Use sysfs_emit() to instead of scnprintf()
platform/x86: dell-smbios-base: Use sysfs_emit()
platform/x86/amd/pmf: Remove unused power_delta instances
platform/x86/amd/pmf: install notify handler after acpi init
Documentation/ABI/testing/sysfs-amd-pmf: Add ABI doc for AMD PMF
platform/x86/amd/pmf: Add sysfs to toggle CnQF
platform/x86/amd/pmf: Add support for CnQF
platform/x86/amd: pmc: Fix build without debugfs
platform/x86: hp-wmi: Support touchpad on/off
platform/x86: int3472/discrete: Drop a forward declaration
platform/x86: toshiba_acpi: change turn_on_panel_on_resume to static
platform/x86: wmi: Drop forward declaration of static functions
platform/x86: toshiba_acpi: Remove duplicate include
platform/x86: msi-laptop: Change DMI match / alias strings to fix module autoloading
...
Add software nodes for the HID sensor collection and the UCM UCSI HID
client to the Surface Pro 8. In contrast to the type-cover devices,
these devices are directly attached to the SAM controller, without any
hub.
This enables support for HID-based sensors, including the ones used for
automatic screen rotation, on the Surface Pro 8.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20220810144117.493710-4-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Add software nodes for the HID sensor collection and the UCM UCSI HID
client to the Surface Pro 8. In contrast to the type-cover devices,
these devices are directly attached to the SAM controller, without any
hub.
This enables support for HID-based sensors, including the ones used for
automatic screen rotation, on the Surface Pro 8.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20220810144117.493710-4-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Move helper functions for client device registration to the core module.
This simplifies addition of future DT/OF support and also allows us to
split out the device hub drivers into their own module.
At the same time, also improve device node validation a bit by not
silently skipping devices with invalid device UID specifiers. Further,
ensure proper lifetime management for the firmware/software nodes
associated with the added devices.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20220624205800.1355621-2-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Add a driver providing a tablet-mode switch input device for Microsoft
Surface devices using the Surface Aggregator KIP subsystem (to manage
detachable peripherals) or POS subsystem (to obtain device posture
information).
The KIP (full name unknown, abbreviation found through reverse
engineering) subsystem is used on the Surface Pro 8 and Surface Pro X to
manage the keyboard cover. Among other things, it provides information
on the positioning (posture) of the cover (closed, laptop-style,
detached, folded-back, ...), which can be used to implement an input
device providing the SW_TABLET_MODE event. Similarly, the POS (posture
information) subsystem provides such information on the Surface Laptop
Studio, with the difference being that the keyboard is not detachable.
As implementing the tablet-mode switch for both subsystems is largely
similar, the driver proposed in this commit, in large, acts as a generic
tablet mode switch driver framework for the Surface Aggregator Module.
Specific implementations using this framework are provided for the KIP
and POS subsystems, adding tablet-mode switch support to the
aforementioned devices.
A few more notes on the Surface Laptop Studio:
A peculiarity of the Surface Laptop Studio is its "slate/tent" mode
(symbolized: user> _/\). In this mode, the screen covers the keyboard
but leaves the touchpad exposed. This is essentially a mode in-between
tablet and laptop, and it is debatable whether tablet-mode should be
enabled in this mode. We therefore let the user decide this via a module
parameter.
In particular, tablet-mode may bring up the on-screen touch keyboard
more easily, which would be desirable in this mode. However, some
user-space software currently also decides to disable keyboard and, more
importantly, touchpad input, while the touchpad is still accessible in
the "slate/tent" mode. Furthermore, this mode shares its identifier with
"slate/flipped" mode where the screen is flipped 180° and the keyboard
points away from the user (symbolized: user> /_). In this mode we would
like to enable auto-rotation, something that user-space software may
only do when tablet-mode is enabled. We therefore default to the
slate-mode enabling the tablet-mode switch.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20220624183642.910893-3-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Add support for the detachable keyboard cover on the Surface Pro 8.
The keyboard cover on the Surface Pro 8 is, unlike the keyboard covers
of earlier Surface Pro generations, handled via the Surface System
Aggregator Module (SSAM). The keyboard and touchpad (as well as other
HID input devices) of this cover are standard SSAM HID client devices
(just like keyboard and touchpad on e.g. the Surface Laptop 3 and 4),
however, some care needs to be taken as they can be physically detached
(similarly to the Surface Book 3). Specifically, the respective SSAM
client devices need to be removed when the keyboard cover has been
detached and (re-)initialized when the keyboard cover has been
(re-)attached.
On the Surface Pro 8, detachment of the keyboard cover (and by extension
its devices) is managed via the KIP subsystem. Therefore, said devices
need to be registered under the KIP device hub, which in turn will
remove and re-create/re-initialize those devices as needed.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20220527023447.2460025-13-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Add a Surface System Aggregator Module (SSAM) client device hub for
hot-removable devices managed via the KIP subsystem.
The KIP subsystem (full name unknown, abbreviation has been obtained
through reverse engineering) is a subsystem that manages hot-removable
SSAM client devices. Specifically, it manages HID input devices
contained in the detachable keyboard cover of the Surface Pro 8 and
Surface Pro X.
The KIP subsystem handles a single group of devices (e.g. all devices
contained in the keyboard cover) and cannot handle devices individually.
Thus we model it as a client device hub, which (hot-)removes all devices
contained under it once removal of the hub (e.g. keyboard cover) has
been detected and (re-)adds all devices once the physical hub device has
been (re-)attached. To do this, use the previously generified SSAM
subsystem hub framework.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20220527023447.2460025-12-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
The Surface System Aggregator Module (SSAM) has multiple subsystems that
can manage detachable devices. At the moment, we only support the "base"
(BAS/0x11) subsystem, which is used on the Surface Book 3 to manage
devices (including keyboard, touchpad, and secondary battery) connected
to the base of the device.
The Surface Pro 8 has a new type-cover with keyboard and touchpad, which
is managed via the KIP/0x0e subsystem. The general procedure is the
same, but with slightly different events and setup. To make
implementation of the KIP hub easier and prevent duplication, generify
the parts of the base hub that we can use for the KIP hub (or any
potential future subsystem hubs).
This also switches over to use the newly introduced "hot-remove"
functionality, which should prevent communication issues when devices
have been detached.
Lastly, also drop the undocumented and unused sysfs "state" attribute of
the base hub. It has at best been useful for debugging.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20220527023447.2460025-10-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
When SSAM client devices have been (physically) hot-removed,
communication attempts with those devices may fail and time out. This
can even extend to event notifiers, due to which timeouts may occur
during device removal, slowing down that process.
Add a parameter to the notifier unregister function that allows skipping
communication with the EC to prevent this. Furthermore, add wrappers for
registering and unregistering notifiers belonging to SSAM client devices
that automatically check if the device has been marked as hot-removed
and communication should be avoided.
Note that non-SSAM client devices can generally not be hot-removed, so
also add a convenience wrapper for those, defaulting to allow
communication.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20220527023447.2460025-4-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
When building the Surface Aggregator Module (SAM) core, registry, and
other SAM client drivers as builtin modules (=y), proper initialization
order is not guaranteed. Due to this, client driver registration
(triggered by device registration in the registry) races against bus
initialization in the core.
If any attempt is made at registering the device driver before the bus
has been initialized (i.e. if bus initialization fails this race) driver
registration will fail with a message similar to:
Driver surface_battery was unable to register with bus_type surface_aggregator because the bus was not initialized
Switch from module_init() to subsys_initcall() to resolve this issue.
Note that the serdev subsystem uses postcore_initcall() so we are still
able to safely register the serdev device driver for the core.
Fixes: c167b9c7e3 ("platform/surface: Add Surface Aggregator subsystem")
Reported-by: Blaž Hrastnik <blaz@mxxn.io>
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20220429195738.535751-1-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>