| Message ID | 20240229-onboard_xvf3500-v6-0-a0aff2947040@wolfvision.net |
|---|---|
| Headers | show |
| Series | usb: misc: onboard_hub: add support for XMOS XVF3500 | expand |
On Thu, Feb 29, 2024 at 09:34:44AM +0100, Javier Carrasco wrote: > Commit 14485de431b0 ("usb: Use device_get_match_data()") overlooked the > already existing pointer to pdev->dev 'dev'. > > Use the existing pointer 'dev' in device_get_match_data() to keep code > consistency. > > Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> Acked-by: Matthias Kaehlcke <mka@chromium.org>
On Thu, Feb 29, 2024 at 09:34:45AM +0100, Javier Carrasco wrote: > The current implementation uses generic names for the power supplies, > which conflicts with proper name definitions in the device bindings. > > Add a per-device property to include real supply names and keep generic > names for existing devices to keep backward compatibility. > > Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> Acked-by: Matthias Kaehlcke <mka@chromium.org>
On Thu, Feb 29, 2024 at 09:34:46AM +0100, Javier Carrasco wrote: > This patch prepares onboad_hub to support non-hub devices by renaming > the driver files and their content, the headers and their references. > > The comments and descriptions have been slightly modified to keep > coherence and account for the specific cases that only affect onboard > hubs (e.g. peer-hub). > > The "hub" variables in functions where "dev" (and similar names) variables > already exist have been renamed to onboard_dev for clarity, which adds a > few lines in cases where more than 80 characters are used. > > No new functionality has been added. > > Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> Acked-by: Matthias Kaehlcke <mka@chromium.org> This should land together with "usb: misc: onboard_dev: add support for non-hub devices". > diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c > new file mode 100644 > index 000000000000..4ae580445408 > --- /dev/null > +++ b/drivers/usb/misc/onboard_usb_dev.c > > ... > > +static const struct usb_device_id onboard_dev_id_table[] = { > + { USB_DEVICE(VENDOR_ID_CYPRESS, 0x6504) }, /* CYUSB33{0,1,2}x/CYUSB230x 3.0 HUB */ > + { USB_DEVICE(VENDOR_ID_CYPRESS, 0x6506) }, /* CYUSB33{0,1,2}x/CYUSB230x 2.0 HUB */ > + { USB_DEVICE(VENDOR_ID_CYPRESS, 0x6570) }, /* CY7C6563x 2.0 HUB */ > + { USB_DEVICE(VENDOR_ID_GENESYS, 0x0608) }, /* Genesys Logic GL850G USB 2.0 HUB */ > + { USB_DEVICE(VENDOR_ID_GENESYS, 0x0610) }, /* Genesys Logic GL852G USB 2.0 HUB */ > + { USB_DEVICE(VENDOR_ID_GENESYS, 0x0620) }, /* Genesys Logic GL3523 USB 3.1 HUB */ > + { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2412) }, /* USB2412 USB 2.0 HUB */ > + { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2514) }, /* USB2514B USB 2.0 HUB */ > + { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2517) }, /* USB2517 USB 2.0 HUB */ > + { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2744) }, /* USB5744 USB 2.0 HUB */ > + { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x5744) }, /* USB5744 USB 3.0 HUB */ > + { USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 HUB */ > + { USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 HUB */ > + { USB_DEVICE(VENDOR_ID_REALTEK, 0x0414) }, /* RTS5414 USB 3.2 HUB */ > + { USB_DEVICE(VENDOR_ID_REALTEK, 0x5414) }, /* RTS5414 USB 2.1 HUB */ > + { USB_DEVICE(VENDOR_ID_TI, 0x8140) }, /* TI USB8041 3.0 HUB */ > + { USB_DEVICE(VENDOR_ID_TI, 0x8142) }, /* TI USB8041 2.0 HUB */ > + { USB_DEVICE(VENDOR_ID_VIA, 0x0817) }, /* VIA VL817 3.1 HUB */ > + { USB_DEVICE(VENDOR_ID_VIA, 0x2817) }, /* VIA VL817 2.0 HUB */ > + {} > +}; nit: 'hub' isn't an acronym, please s/HUB/hub/ in the next revision.
On Thu, Feb 29, 2024 at 09:34:47AM +0100, 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. > > Acked-by: Helen Koike <helen.koike@collabora.com> > Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
On Thu, Feb 29, 2024 at 09:34:48AM +0100, 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 <javier.carrasco@wolfvision.net> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
On Thu, Feb 29, 2024 at 09:34:49AM +0100, 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 accordingly. > > Update to the new name (ONBOARD_USB_DEV) accordingly. > > Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
On Thu, Feb 29, 2024 at 09:34:50AM +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. > > The 'always_powered_in_supend' attribute is only available for hub > devices, keeping the driver's default behavior for non-hub devices (keep > on in suspend). > > Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> > --- > drivers/usb/misc/onboard_usb_dev.c | 25 ++++++++++++++++++++++++- > drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++ > 2 files changed, 34 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c > index 4ae580445408..f1b174503c44 100644 > --- a/drivers/usb/misc/onboard_usb_dev.c > +++ b/drivers/usb/misc/onboard_usb_dev.c > @@ -261,7 +261,27 @@ static struct attribute *onboard_dev_attrs[] = { > &dev_attr_always_powered_in_suspend.attr, > NULL, > }; > -ATTRIBUTE_GROUPS(onboard_dev); > + > +static umode_t onboard_dev_attrs_are_visible(struct kobject *kobj, > + struct attribute *attr, > + int n) > +{ > + struct device *dev = kobj_to_dev(kobj); > + struct onboard_dev *onboard_dev = dev_get_drvdata(dev); > + > + if (attr == &dev_attr_always_powered_in_suspend.attr && > + !onboard_dev->pdata->is_hub) > + return 0; > + > + return attr->mode; > +} > + > +static const struct attribute_group onboard_dev_group = { > + .is_visible = onboard_dev_attrs_are_visible, > + .attrs = onboard_dev_attrs, > +}; > +__ATTRIBUTE_GROUPS(onboard_dev); > + nit: remove one empty line > > static void onboard_dev_attach_usb_driver(struct work_struct *work) > { > @@ -286,6 +306,9 @@ static int onboard_dev_probe(struct platform_device *pdev) > if (!onboard_dev->pdata) > return -EINVAL; > > + if (!onboard_dev->pdata->is_hub) > + onboard_dev->always_powered_in_suspend = true; > + > onboard_dev->dev = dev; > > err = onboard_dev_get_regulators(onboard_dev); > diff --git a/drivers/usb/misc/onboard_usb_dev.h b/drivers/usb/misc/onboard_usb_dev.h > index 4da9f3b7f9e9..58cf8c81b2cf 100644 > --- a/drivers/usb/misc/onboard_usb_dev.h > +++ b/drivers/usb/misc/onboard_usb_dev.h > @@ -12,60 +12,70 @@ struct onboard_dev_pdata { > unsigned long reset_us; /* reset pulse width in us */ > unsigned int num_supplies; /* number of supplies */ > const char * const supply_names[MAX_SUPPLIES]; > + bool is_hub; /* true if the device is a HUB */ nit: either drop the comment (the variable name is pretty self explaining), or s/HUB/hub/ ('hub' isn't an acronym). Acked-by: Matthias Kaehlcke <mka@chromium.org>
On Thu, Feb 29, 2024 at 09:34:52AM +0100, Javier Carrasco wrote: > 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. > > 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 <javier.carrasco@wolfvision.net> Acked-by: Matthias Kaehlcke <mka@chromium.org>
On 29.02.24 20:52, Matthias Kaehlcke wrote: > On Thu, Feb 29, 2024 at 09:34:50AM +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. >> >> The 'always_powered_in_supend' attribute is only available for hub >> devices, keeping the driver's default behavior for non-hub devices (keep >> on in suspend). >> >> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> >> --- >> drivers/usb/misc/onboard_usb_dev.c | 25 ++++++++++++++++++++++++- >> drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++ >> 2 files changed, 34 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c >> index 4ae580445408..f1b174503c44 100644 >> --- a/drivers/usb/misc/onboard_usb_dev.c >> +++ b/drivers/usb/misc/onboard_usb_dev.c >> @@ -261,7 +261,27 @@ static struct attribute *onboard_dev_attrs[] = { >> &dev_attr_always_powered_in_suspend.attr, >> NULL, >> }; >> -ATTRIBUTE_GROUPS(onboard_dev); >> + >> +static umode_t onboard_dev_attrs_are_visible(struct kobject *kobj, >> + struct attribute *attr, >> + int n) >> +{ >> + struct device *dev = kobj_to_dev(kobj); >> + struct onboard_dev *onboard_dev = dev_get_drvdata(dev); >> + >> + if (attr == &dev_attr_always_powered_in_suspend.attr && >> + !onboard_dev->pdata->is_hub) >> + return 0; >> + >> + return attr->mode; >> +} >> + >> +static const struct attribute_group onboard_dev_group = { >> + .is_visible = onboard_dev_attrs_are_visible, >> + .attrs = onboard_dev_attrs, >> +}; >> +__ATTRIBUTE_GROUPS(onboard_dev); >> + > > nit: remove one empty line > >> >> static void onboard_dev_attach_usb_driver(struct work_struct *work) >> { >> @@ -286,6 +306,9 @@ static int onboard_dev_probe(struct platform_device *pdev) >> if (!onboard_dev->pdata) >> return -EINVAL; >> >> + if (!onboard_dev->pdata->is_hub) >> + onboard_dev->always_powered_in_suspend = true; >> + >> onboard_dev->dev = dev; >> >> err = onboard_dev_get_regulators(onboard_dev); >> diff --git a/drivers/usb/misc/onboard_usb_dev.h b/drivers/usb/misc/onboard_usb_dev.h >> index 4da9f3b7f9e9..58cf8c81b2cf 100644 >> --- a/drivers/usb/misc/onboard_usb_dev.h >> +++ b/drivers/usb/misc/onboard_usb_dev.h >> @@ -12,60 +12,70 @@ struct onboard_dev_pdata { >> unsigned long reset_us; /* reset pulse width in us */ >> unsigned int num_supplies; /* number of supplies */ >> const char * const supply_names[MAX_SUPPLIES]; >> + bool is_hub; /* true if the device is a HUB */ > > nit: either drop the comment (the variable name is pretty self explaining), > or s/HUB/hub/ ('hub' isn't an acronym). > > Acked-by: Matthias Kaehlcke <mka@chromium.org> To be honest, I added the description to follow the same pattern used for the previous fields: unsigned long reset_us; /* reset pulse width in us */ unsigned int num_supplies; /* number of supplies */ Best regards, 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. 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 also been added, keeping generic names for already supported devices to keep backwards compatibility. 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. 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 <lgirdwood@gmail.com> To: Mark Brown <broonie@kernel.org> To: Rob Herring <robh+dt@kernel.org> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> To: Conor Dooley <conor+dt@kernel.org> To: Matthias Kaehlcke <mka@chromium.org> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org> To: Helen Koike <helen.koike@collabora.com> To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> To: Maxime Ripard <mripard@kernel.org> To: Thomas Zimmermann <tzimmermann@suse.de> To: David Airlie <airlied@gmail.com> To: Daniel Vetter <daniel@ffwll.ch> To: Catalin Marinas <catalin.marinas@arm.com> To: Will Deacon <will@kernel.org> To: Russell King <linux@armlinux.org.uk> Cc: linux-sound@vger.kernel.org Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-usb@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> Changes in v6: - onboard_usb_hub.c: use dev pointer in probe consistently (new patch). - onboard_usb_hub.c: rename get_regulator_bulk function to get_regulators and only pass onboard_hub (hub in probe) as argument. - onboard_usb_hub.c: drop file after renaming. - onboard_usb_dev.c: improve device descriptions in usb_device_id table. - onboard_usb_dev.c: keep non-hub devices powered on in suspend. - General: update commit messages (use usb_hub_dev after renaming). - Link to v5: https://lore.kernel.org/r/20240228-onboard_xvf3500-v5-0-76b805fd3fe6@wolfvision.net Changes in v5: - onboard_usb_dev: move device suppy names handling to [1/8]. - onboard_usb_dev.c: make always_powered_in_suspend not visible for non-hub devices. - onboard_usb_dev.c: move is_hub check in suspend() to functio entry. - onboard_usb_dev_pdevs.c: comment rephrasing to account for hub-specific attribute. - Link to v4: https://lore.kernel.org/r/20240220-onboard_xvf3500-v4-0-dc1617cc5dd4@wolfvision.net 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/20240206-onboard_xvf3500-v3-0-f85b04116688@wolfvision.net 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/20240130-onboard_xvf3500-v1-0-51b5398406cb@wolfvision.net 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/20240115-feature-xvf3500_driver-v1-0-ed9cfb48bb85@wolfvision.net/ --- Javier Carrasco (9): usb: misc: onboard_hub: use pointer consistently in the probe function usb: misc: onboard_hub: use device supply names usb: misc: onboard_hub: rename to onboard_dev 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 to ONBOAD_USB_DEV usb: misc: onboard_dev: add support for non-hub devices ASoC: dt-bindings: xmos,xvf3500: add XMOS XVF3500 voice processor usb: misc: onboard_dev: add support for XMOS XVF3500 ...-usb-hub => sysfs-bus-platform-onboard-usb-dev} | 3 +- .../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 | 544 +++++++++++++++++++++ .../misc/{onboard_usb_hub.h => onboard_usb_dev.h} | 58 ++- ...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, 717 insertions(+), 579 deletions(-) --- base-commit: 41bccc98fb7931d63d03f326a746ac4d429c1dd3 change-id: 20240130-onboard_xvf3500-6c0e78d11a1b Best regards,