This series adds support for the XMOS XVF3500 VocalFusion Voice
Processor[1], a low-latency, 32-bit multicore controller for voice
processing.
The XVF3500 requires a specific power sequence, which consists of
enabling the regulators that control the 3V3 and 1V0 device supplies,
and a reset de-assertion after a delay of at least 100ns. Once in normal
operation, the XVF3500 registers itself as a regular USB device and no
device-specific management is required.
The power management provided by onboard_usb_hub is not specific for hubs
and any other USB device with the same power sequence could profit from
that driver, provided that the device does not have any specific
requirements beyond the power management. To account for non-hub devices,
the driver has been renamed and an extra flag has been added to identify
hubs and provide their specific functionality. Support for
device-specific power suply names has been added, keeping generic names
for backwards compatibility and as a fallback mechanism.
The references to onboard_usb_hub in the core and config files have been
updated as well.
The diff is way much bulkier than the actual code addition because of the
file renaming, so in order to ease reviews and catch hub-specific code
that might still affect non-hub devices, the complete renaming was moved
to a single commit.
The device binding has been added to sound/ because it is the subsystem
that covers its functionality (voice processing) during normal
operation. If it should reside under usb/ instead, it will be moved as
required.
This series has been tested with a Rockchip-based SoC and an XMOS
XVF3500-FB167-C.
[1] https://www.xmos.com/xvf3500/
To: Liam Girdwood <[email protected]>
To: Mark Brown <[email protected]>
To: Rob Herring <[email protected]>
To: Krzysztof Kozlowski <[email protected]>
To: Conor Dooley <[email protected]>
To: Matthias Kaehlcke <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Helen Koike <[email protected]>
To: Maarten Lankhorst <[email protected]>
To: Maxime Ripard <[email protected]>
To: Thomas Zimmermann <[email protected]>
To: David Airlie <[email protected]>
To: Daniel Vetter <[email protected]>
To: Catalin Marinas <[email protected]>
To: Will Deacon <[email protected]>
To: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Javier Carrasco <[email protected]>
Changes in v4:
- General: use device supply names and generics as fallback.
- onbord_usb_dev.c: fix suspend callback for non-hub devices.
- onboard_usb_dev.c: fix typos.
- Link to v3: https://lore.kernel.org/r/[email protected]
Changes in v3:
- onboard_usb_hub: rename to onboard_usb_dev to include non-hub devices.
- onboard_hub_dev: add flag to identify hubs and provide their extra
functionality.
- dt-bindings: add reference to usb-device.yaml and usb node in the
example.
- dt-bindings: generic node name.
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
- general: add support in onboard_usb_hub instead of using a dedicated
driver.
- dt-bindings: use generic usb-device compatible ("usbVID,PID").
- Link to v1: https://lore.kernel.org/all/[email protected]/
---
Javier Carrasco (8):
usb: misc: onboard_hub: rename to onboard_dev
usb: misc: onboard_dev: add support for non-hub devices
drm: ci: arm64.config: update ONBOARD_USB_HUB to ONBOARD_USB_DEV
arm64: defconfig: update ONBOARD_USB_HUB to ONBOARD_USB_DEV
ARM: multi_v7_defconfig: update ONBOARD_USB_HUB name
usb: misc: onboard_dev: use device supply names
ASoC: dt-bindings: xmos,xvf3500: add XMOS XVF3500 voice processor
usb: misc: onboard_hub: add support for XMOS XVF3500
...-usb-hub => sysfs-bus-platform-onboard-usb-dev} | 4 +-
.../devicetree/bindings/sound/xmos,xvf3500.yaml | 63 +++
MAINTAINERS | 4 +-
arch/arm/configs/multi_v7_defconfig | 2 +-
arch/arm64/configs/defconfig | 2 +-
drivers/gpu/drm/ci/arm64.config | 4 +-
drivers/usb/core/Makefile | 4 +-
drivers/usb/core/hub.c | 8 +-
drivers/usb/core/hub.h | 2 +-
drivers/usb/misc/Kconfig | 16 +-
drivers/usb/misc/Makefile | 2 +-
drivers/usb/misc/onboard_usb_dev.c | 525 +++++++++++++++++++++
.../misc/{onboard_usb_hub.h => onboard_usb_dev.h} | 69 ++-
...ard_usb_hub_pdevs.c => onboard_usb_dev_pdevs.c} | 47 +-
drivers/usb/misc/onboard_usb_hub.c | 501 --------------------
include/linux/usb/onboard_dev.h | 18 +
include/linux/usb/onboard_hub.h | 18 -
17 files changed, 709 insertions(+), 580 deletions(-)
---
base-commit: 41bccc98fb7931d63d03f326a746ac4d429c1dd3
change-id: 20240130-onboard_xvf3500-6c0e78d11a1b
Best regards,
--
Javier Carrasco <[email protected]>
Most of the functionality this driver provides can be used by non-hub
devices as well.
To account for the hub-specific code, add a flag to the device data
structure and check its value for hub-specific code.
Signed-off-by: Javier Carrasco <[email protected]>
---
drivers/usb/misc/onboard_usb_dev.c | 3 ++-
drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c
index 2103af2cb2a6..f43130a6786f 100644
--- a/drivers/usb/misc/onboard_usb_dev.c
+++ b/drivers/usb/misc/onboard_usb_dev.c
@@ -129,7 +129,8 @@ static int __maybe_unused onboard_dev_suspend(struct device *dev)
if (!device_may_wakeup(node->udev->bus->controller))
continue;
- if (usb_wakeup_enabled_descendants(node->udev)) {
+ if (usb_wakeup_enabled_descendants(node->udev) ||
+ !onboard_dev->pdata->is_hub) {
power_off = false;
break;
}
diff --git a/drivers/usb/misc/onboard_usb_dev.h b/drivers/usb/misc/onboard_usb_dev.h
index f13d11a84371..ebe83e19d818 100644
--- a/drivers/usb/misc/onboard_usb_dev.h
+++ b/drivers/usb/misc/onboard_usb_dev.h
@@ -9,51 +9,61 @@
struct onboard_dev_pdata {
unsigned long reset_us; /* reset pulse width in us */
unsigned int num_supplies; /* number of supplies */
+ bool is_hub;
};
static const struct onboard_dev_pdata microchip_usb424_data = {
.reset_us = 1,
.num_supplies = 1,
+ .is_hub = true,
};
static const struct onboard_dev_pdata microchip_usb5744_data = {
.reset_us = 0,
.num_supplies = 2,
+ .is_hub = true,
};
static const struct onboard_dev_pdata realtek_rts5411_data = {
.reset_us = 0,
.num_supplies = 1,
+ .is_hub = true,
};
static const struct onboard_dev_pdata ti_tusb8041_data = {
.reset_us = 3000,
.num_supplies = 1,
+ .is_hub = true,
};
static const struct onboard_dev_pdata cypress_hx3_data = {
.reset_us = 10000,
.num_supplies = 2,
+ .is_hub = true,
};
static const struct onboard_dev_pdata cypress_hx2vl_data = {
.reset_us = 1,
.num_supplies = 1,
+ .is_hub = true,
};
static const struct onboard_dev_pdata genesys_gl850g_data = {
.reset_us = 3,
.num_supplies = 1,
+ .is_hub = true,
};
static const struct onboard_dev_pdata genesys_gl852g_data = {
.reset_us = 50,
.num_supplies = 1,
+ .is_hub = true,
};
static const struct onboard_dev_pdata vialab_vl817_data = {
.reset_us = 10,
.num_supplies = 1,
+ .is_hub = true,
};
static const struct of_device_id onboard_dev_match[] = {
--
2.40.1
The onboard_usb_hub driver has been updated to support non-hub devices,
which has led to some renaming.
Update to the new name (ONBOARD_USB_DEV) accordingly.
Signed-off-by: Javier Carrasco <[email protected]>
---
drivers/gpu/drm/ci/arm64.config | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ci/arm64.config b/drivers/gpu/drm/ci/arm64.config
index 8dbce9919a57..4140303d6260 100644
--- a/drivers/gpu/drm/ci/arm64.config
+++ b/drivers/gpu/drm/ci/arm64.config
@@ -87,7 +87,7 @@ CONFIG_DRM_PARADE_PS8640=y
CONFIG_DRM_LONTIUM_LT9611UXC=y
CONFIG_PHY_QCOM_USB_HS=y
CONFIG_QCOM_GPI_DMA=y
-CONFIG_USB_ONBOARD_HUB=y
+CONFIG_USB_ONBOARD_DEV=y
CONFIG_NVMEM_QCOM_QFPROM=y
CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2=y
@@ -97,7 +97,7 @@ CONFIG_USB_RTL8152=y
# db820c ethernet
CONFIG_ATL1C=y
# Chromebooks ethernet
-CONFIG_USB_ONBOARD_HUB=y
+CONFIG_USB_ONBOARD_DEV=y
# 888 HDK ethernet
CONFIG_USB_LAN78XX=y
--
2.40.1
The onboard_usb_hub driver has been updated to support non-hub devices,
which has led to some renaming. Update to the new name accordingly.
Update to the new name (ONBOARD_USB_DEV) accordingly.
Signed-off-by: Javier Carrasco <[email protected]>
---
arch/arm/configs/multi_v7_defconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index ecb3e286107a..6a6ebec173dc 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -876,7 +876,7 @@ CONFIG_USB_CHIPIDEA_UDC=y
CONFIG_USB_CHIPIDEA_HOST=y
CONFIG_USB_ISP1760=y
CONFIG_USB_HSIC_USB3503=y
-CONFIG_USB_ONBOARD_HUB=m
+CONFIG_USB_ONBOARD_DEV=m
CONFIG_AB8500_USB=y
CONFIG_KEYSTONE_USB_PHY=m
CONFIG_NOP_USB_XCEIV=y
--
2.40.1
The XMOS XVF3500 VocalFusion Voice Processor[1] is a low-latency, 32-bit
multicore controller for voice processing.
This device requires a specific power sequence, which consists of
enabling the regulators that control the 3V3 and 1V0 device supplies,
and a reset de-assertion after a delay of at least 100ns. Such power
sequence is already supported by the onboard_hub driver, and it can be
reused for non-hub USB devices as well.
Once in normal operation, the XVF3500 registers itself as a USB device,
and it does not require any device-specific operations in the driver.
[1] https://www.xmos.com/xvf3500/
Signed-off-by: Javier Carrasco <[email protected]>
---
drivers/usb/misc/onboard_usb_dev.c | 2 ++
drivers/usb/misc/onboard_usb_dev.h | 8 ++++++++
2 files changed, 10 insertions(+)
diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c
index e66fcac93006..e717d1ca8d79 100644
--- a/drivers/usb/misc/onboard_usb_dev.c
+++ b/drivers/usb/misc/onboard_usb_dev.c
@@ -388,6 +388,7 @@ static struct platform_driver onboard_dev_driver = {
#define VENDOR_ID_REALTEK 0x0bda
#define VENDOR_ID_TI 0x0451
#define VENDOR_ID_VIA 0x2109
+#define VENDOR_ID_XMOS 0x20B1
/*
* Returns the onboard_dev platform device that is associated with the USB
@@ -480,6 +481,7 @@ static const struct usb_device_id onboard_dev_id_table[] = {
{ USB_DEVICE(VENDOR_ID_TI, 0x8142) }, /* TI USB8041 2.0 */
{ USB_DEVICE(VENDOR_ID_VIA, 0x0817) }, /* VIA VL817 3.1 */
{ USB_DEVICE(VENDOR_ID_VIA, 0x2817) }, /* VIA VL817 2.0 */
+ { USB_DEVICE(VENDOR_ID_XMOS, 0x0013) }, /* XVF3500 */
{}
};
MODULE_DEVICE_TABLE(usb, onboard_dev_id_table);
diff --git a/drivers/usb/misc/onboard_usb_dev.h b/drivers/usb/misc/onboard_usb_dev.h
index 59dced6bd339..921a5276fc7f 100644
--- a/drivers/usb/misc/onboard_usb_dev.h
+++ b/drivers/usb/misc/onboard_usb_dev.h
@@ -89,6 +89,13 @@ static const struct onboard_dev_pdata vialab_vl817_data = {
.is_hub = true,
};
+static const struct onboard_dev_pdata xmos_xvf3500_data = {
+ .reset_us = 1,
+ .num_supplies = 2,
+ .supply_names = { "vdd", "vddio" },
+ .is_hub = false,
+};
+
static const struct of_device_id onboard_dev_match[] = {
{ .compatible = "usb424,2412", .data = µchip_usb424_data, },
{ .compatible = "usb424,2514", .data = µchip_usb424_data, },
@@ -110,6 +117,7 @@ static const struct of_device_id onboard_dev_match[] = {
{ .compatible = "usbbda,5414", .data = &realtek_rts5411_data, },
{ .compatible = "usb2109,817", .data = &vialab_vl817_data, },
{ .compatible = "usb2109,2817", .data = &vialab_vl817_data, },
+ { .compatible = "usb20b1,0013", .data = &xmos_xvf3500_data, },
{}
};
--
2.40.1
On 20/02/2024 11:05, Javier Carrasco wrote:
> The onboard_usb_hub driver has been updated to support non-hub devices,
> which has led to some renaming.
>
> Update to the new name (ONBOARD_USB_DEV) accordingly.
>
> Signed-off-by: Javier Carrasco <[email protected]>
Acked-by: Helen Koike <[email protected]>
Regards,
Helen
> ---
> drivers/gpu/drm/ci/arm64.config | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/ci/arm64.config b/drivers/gpu/drm/ci/arm64.config
> index 8dbce9919a57..4140303d6260 100644
> --- a/drivers/gpu/drm/ci/arm64.config
> +++ b/drivers/gpu/drm/ci/arm64.config
> @@ -87,7 +87,7 @@ CONFIG_DRM_PARADE_PS8640=y
> CONFIG_DRM_LONTIUM_LT9611UXC=y
> CONFIG_PHY_QCOM_USB_HS=y
> CONFIG_QCOM_GPI_DMA=y
> -CONFIG_USB_ONBOARD_HUB=y
> +CONFIG_USB_ONBOARD_DEV=y
> CONFIG_NVMEM_QCOM_QFPROM=y
> CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2=y
>
> @@ -97,7 +97,7 @@ CONFIG_USB_RTL8152=y
> # db820c ethernet
> CONFIG_ATL1C=y
> # Chromebooks ethernet
> -CONFIG_USB_ONBOARD_HUB=y
> +CONFIG_USB_ONBOARD_DEV=y
> # 888 HDK ethernet
> CONFIG_USB_LAN78XX=y
>
>
Hi,
Am Dienstag, 20. Februar 2024, 15:05:44 CET schrieb Javier Carrasco:
> This series adds support for the XMOS XVF3500 VocalFusion Voice
> Processor[1], a low-latency, 32-bit multicore controller for voice
> processing.
>
> The XVF3500 requires a specific power sequence, which consists of
> enabling the regulators that control the 3V3 and 1V0 device supplies,
> and a reset de-assertion after a delay of at least 100ns. Once in normal
> operation, the XVF3500 registers itself as a regular USB device and no
> device-specific management is required.
While reading this, [1] come into my mind.
Best regards,
Alexander
[1] https://lore.kernel.org/all/[email protected]/
> The power management provided by onboard_usb_hub is not specific for hubs
> and any other USB device with the same power sequence could profit from
> that driver, provided that the device does not have any specific
> requirements beyond the power management. To account for non-hub devices,
> the driver has been renamed and an extra flag has been added to identify
> hubs and provide their specific functionality. Support for
> device-specific power suply names has been added, keeping generic names
> for backwards compatibility and as a fallback mechanism.
>
> The references to onboard_usb_hub in the core and config files have been
> updated as well.
>
> The diff is way much bulkier than the actual code addition because of the
> file renaming, so in order to ease reviews and catch hub-specific code
> that might still affect non-hub devices, the complete renaming was moved
> to a single commit.
>
> The device binding has been added to sound/ because it is the subsystem
> that covers its functionality (voice processing) during normal
> operation. If it should reside under usb/ instead, it will be moved as
> required.
>
> This series has been tested with a Rockchip-based SoC and an XMOS
> XVF3500-FB167-C.
>
> [1] https://www.xmos.com/xvf3500/
>
> To: Liam Girdwood <[email protected]>
> To: Mark Brown <[email protected]>
> To: Rob Herring <[email protected]>
> To: Krzysztof Kozlowski <[email protected]>
> To: Conor Dooley <[email protected]>
> To: Matthias Kaehlcke <[email protected]>
> To: Greg Kroah-Hartman <[email protected]>
> To: Helen Koike <[email protected]>
> To: Maarten Lankhorst <[email protected]>
> To: Maxime Ripard <[email protected]>
> To: Thomas Zimmermann <[email protected]>
> To: David Airlie <[email protected]>
> To: Daniel Vetter <[email protected]>
> To: Catalin Marinas <[email protected]>
> To: Will Deacon <[email protected]>
> To: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Javier Carrasco <[email protected]>
>
> Changes in v4:
> - General: use device supply names and generics as fallback.
> - onbord_usb_dev.c: fix suspend callback for non-hub devices.
> - onboard_usb_dev.c: fix typos.
>
> - Link to v3: https://lore.kernel.org/r/[email protected]
>
> Changes in v3:
> - onboard_usb_hub: rename to onboard_usb_dev to include non-hub devices.
> - onboard_hub_dev: add flag to identify hubs and provide their extra
> functionality.
> - dt-bindings: add reference to usb-device.yaml and usb node in the
> example.
> - dt-bindings: generic node name.
> - Link to v2: https://lore.kernel.org/r/[email protected]
>
> Changes in v2:
> - general: add support in onboard_usb_hub instead of using a dedicated
> driver.
> - dt-bindings: use generic usb-device compatible ("usbVID,PID").
> - Link to v1: https://lore.kernel.org/all/[email protected]/
>
> ---
> Javier Carrasco (8):
> usb: misc: onboard_hub: rename to onboard_dev
> usb: misc: onboard_dev: add support for non-hub devices
> drm: ci: arm64.config: update ONBOARD_USB_HUB to ONBOARD_USB_DEV
> arm64: defconfig: update ONBOARD_USB_HUB to ONBOARD_USB_DEV
> ARM: multi_v7_defconfig: update ONBOARD_USB_HUB name
> usb: misc: onboard_dev: use device supply names
> ASoC: dt-bindings: xmos,xvf3500: add XMOS XVF3500 voice processor
> usb: misc: onboard_hub: add support for XMOS XVF3500
>
> ...-usb-hub => sysfs-bus-platform-onboard-usb-dev} | 4 +-
> .../devicetree/bindings/sound/xmos,xvf3500.yaml | 63 +++
> MAINTAINERS | 4 +-
> arch/arm/configs/multi_v7_defconfig | 2 +-
> arch/arm64/configs/defconfig | 2 +-
> drivers/gpu/drm/ci/arm64.config | 4 +-
> drivers/usb/core/Makefile | 4 +-
> drivers/usb/core/hub.c | 8 +-
> drivers/usb/core/hub.h | 2 +-
> drivers/usb/misc/Kconfig | 16 +-
> drivers/usb/misc/Makefile | 2 +-
> drivers/usb/misc/onboard_usb_dev.c | 525 +++++++++++++++++++++
> .../misc/{onboard_usb_hub.h => onboard_usb_dev.h} | 69 ++-
> ...ard_usb_hub_pdevs.c => onboard_usb_dev_pdevs.c} | 47 +-
> drivers/usb/misc/onboard_usb_hub.c | 501 --------------------
> include/linux/usb/onboard_dev.h | 18 +
> include/linux/usb/onboard_hub.h | 18 -
> 17 files changed, 709 insertions(+), 580 deletions(-)
> ---
> base-commit: 41bccc98fb7931d63d03f326a746ac4d429c1dd3
> change-id: 20240130-onboard_xvf3500-6c0e78d11a1b
>
> Best regards,
>
--
TQ-Systems GmbH | M?hlstra?e 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht M?nchen, HRB 105018
Gesch?ftsf?hrer: Detlef Schneider, R?diger Stahl, Stefan Schneider
http://www.tq-group.com/
On Tue, Feb 20, 2024 at 03:05:46PM +0100, Javier Carrasco wrote:
> Most of the functionality this driver provides can be used by non-hub
> devices as well.
>
> To account for the hub-specific code, add a flag to the device data
> structure and check its value for hub-specific code.
Please mention that the driver doesn't power off non-hub devices
during system suspend.
> Signed-off-by: Javier Carrasco <[email protected]>
> ---
> drivers/usb/misc/onboard_usb_dev.c | 3 ++-
> drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c
> index 2103af2cb2a6..f43130a6786f 100644
> --- a/drivers/usb/misc/onboard_usb_dev.c
> +++ b/drivers/usb/misc/onboard_usb_dev.c
> @@ -129,7 +129,8 @@ static int __maybe_unused onboard_dev_suspend(struct device *dev)
> if (!device_may_wakeup(node->udev->bus->controller))
> continue;
>
> - if (usb_wakeup_enabled_descendants(node->udev)) {
> + if (usb_wakeup_enabled_descendants(node->udev) ||
> + !onboard_dev->pdata->is_hub) {
This check isn't dependent on characteristics of the USB devices processed
in this loop, therefore it can be performed at function entry. Please combine
it with the check of 'always_powered_in_suspend'. It's also an option to
omit the check completely, 'always_powered_in_suspend' will never be set for
non-hub devices (assuming the sysfs attribute isn't added).
> power_off = false;
> break;
> }
Without code context: please omit the creation of the 'always_powered_in_suspend'
attribute for non-hub devices. As per above we don't plan to hone it, so it
shouldn't exist.
On 21.02.24 20:24, Matthias Kaehlcke wrote:
> On Tue, Feb 20, 2024 at 03:05:46PM +0100, Javier Carrasco wrote:
>> Most of the functionality this driver provides can be used by non-hub
>> devices as well.
>>
>> To account for the hub-specific code, add a flag to the device data
>> structure and check its value for hub-specific code.
>
> Please mention that the driver doesn't power off non-hub devices
> during system suspend.
>
>> Signed-off-by: Javier Carrasco <[email protected]>
>> ---
>> drivers/usb/misc/onboard_usb_dev.c | 3 ++-
>> drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++
>> 2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c
>> index 2103af2cb2a6..f43130a6786f 100644
>> --- a/drivers/usb/misc/onboard_usb_dev.c
>> +++ b/drivers/usb/misc/onboard_usb_dev.c
>> @@ -129,7 +129,8 @@ static int __maybe_unused onboard_dev_suspend(struct device *dev)
>> if (!device_may_wakeup(node->udev->bus->controller))
>> continue;
>>
>> - if (usb_wakeup_enabled_descendants(node->udev)) {
>> + if (usb_wakeup_enabled_descendants(node->udev) ||
>> + !onboard_dev->pdata->is_hub) {
>
>
> This check isn't dependent on characteristics of the USB devices processed
> in this loop, therefore it can be performed at function entry. Please combine
> it with the check of 'always_powered_in_suspend'. It's also an option to
> omit the check completely, 'always_powered_in_suspend' will never be set for
> non-hub devices (assuming the sysfs attribute isn't added).
>
The attribute will not be available for non-hub devices in v5. However,
if the check is completely removed, will power_off not stay true at the
end of the function, always leading to a device power off? As you said,
'always_powered_in_suspend' will not be set for non-hub devices.
>> power_off = false;
>> break;
>> }
>
> Without code context: please omit the creation of the 'always_powered_in_suspend'
> attribute for non-hub devices. As per above we don't plan to hone it, so it
> shouldn't exist.
Best regards,
Javier Carrasco
On Thu, Feb 22, 2024 at 03:42:26PM +0100, Javier Carrasco wrote:
> On 21.02.24 20:24, Matthias Kaehlcke wrote:
> > On Tue, Feb 20, 2024 at 03:05:46PM +0100, Javier Carrasco wrote:
> >> Most of the functionality this driver provides can be used by non-hub
> >> devices as well.
> >>
> >> To account for the hub-specific code, add a flag to the device data
> >> structure and check its value for hub-specific code.
> >
> > Please mention that the driver doesn't power off non-hub devices
> > during system suspend.
> >
> >> Signed-off-by: Javier Carrasco <[email protected]>
> >> ---
> >> drivers/usb/misc/onboard_usb_dev.c | 3 ++-
> >> drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++
> >> 2 files changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c
> >> index 2103af2cb2a6..f43130a6786f 100644
> >> --- a/drivers/usb/misc/onboard_usb_dev.c
> >> +++ b/drivers/usb/misc/onboard_usb_dev.c
> >> @@ -129,7 +129,8 @@ static int __maybe_unused onboard_dev_suspend(struct device *dev)
> >> if (!device_may_wakeup(node->udev->bus->controller))
> >> continue;
> >>
> >> - if (usb_wakeup_enabled_descendants(node->udev)) {
> >> + if (usb_wakeup_enabled_descendants(node->udev) ||
> >> + !onboard_dev->pdata->is_hub) {
> >
> >
> > This check isn't dependent on characteristics of the USB devices processed
> > in this loop, therefore it can be performed at function entry. Please combine
> > it with the check of 'always_powered_in_suspend'. It's also an option to
> > omit the check completely, 'always_powered_in_suspend' will never be set for
> > non-hub devices (assuming the sysfs attribute isn't added).
> >
>
> The attribute will not be available for non-hub devices in v5. However,
> if the check is completely removed, will power_off not stay true at the
> end of the function, always leading to a device power off? As you said,
> 'always_powered_in_suspend' will not be set for non-hub devices.
Even without the sysfs attribute the field 'always_powered_in_suspend' could
be set to true by probe() for non-hub devices.