hid-mcp2200: added driver for GPIOs of MCP2200
PATCH v5 1/1 hid-mcp2200: added driver for GPIOs of MCP2200
From: Johannes Roith <[email protected]>
Added a gpiochip compatible driver to control the 8 GPIOs of the MCP2200 by using the HID interface.
Using GPIOs with alternative functions (GP0<->SSPND, GP1<->USBCFG, GP6<->RXLED, GP7<->TXLED) will reset the functions, if set (unset by default).
The driver was tested while also using the UART of the chip. Setting and reading the GPIOs has no effect on the UART communication. However, a reset is triggered after the CONFIGURE command. If the GPIO Direction is constantly changed, this will affect the communication at low baud rates. This is a hardware problem of the MCP2200 and is not caused by the driver.
Feedback from reviewers Christophe JAILLET <[email protected]> and Andi Shyti <[email protected]> was added.
Changelog:
- v1: added driver
- v2: added ProductID in hid-ids.h
- v3: added feedback from review and make patch compilable
- v4: added more feedback from second review, tested as module and builtin on x86 and arm64
- v5: adopted Kconfig to style guidelines, fixed mcp_set_direction
--- drivers/hid/Kconfig | 10 + drivers/hid/Makefile | 1 + drivers/hid/hid-ids.h | 1 + drivers/hid/hid-mcp2200.c | 419 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 431 insertions(+) create mode 100644 drivers/hid/hid-mcp2200.c
drivers/hid/Kconfig +10 -0
@@ -1301,6 +1301,16 @@ config HID_MCP2221 To compile this driver as a module, choose M here: the module will be called hid-mcp2221.ko. +config HID_MCP2200 + tristate "Microchip MCP2200 HID USB-to-GPIO bridge" + depends on USB_HID + imply GPIOLIB + help + Provides GPIO functionality over USB-HID through MCP2200 device. + + To compile this driver as a module, choose M here: the module + will be called hid-mcp2200.ko. + config HID_KUNIT_TEST tristate "KUnit tests for HID" if !KUNIT_ALL_TESTS depends on KUNIT
drivers/hid/Makefile +1 -0
@@ -79,6 +79,7 @@ obj-$(CONFIG_HID_MACALLY) += hid-macally.o obj-$(CONFIG_HID_MAGICMOUSE) += hid-magicmouse.o obj-$(CONFIG_HID_MALTRON) += hid-maltron.o obj-$(CONFIG_HID_MCP2221) += hid-mcp2221.o +obj-$(CONFIG_HID_MCP2200) += hid-mcp2200.o obj-$(CONFIG_HID_MAYFLASH) += hid-mf.o obj-$(CONFIG_HID_MEGAWORLD_FF) += hid-megaworld.o obj-$(CONFIG_HID_MICROSOFT) += hid-microsoft.o
drivers/hid/hid-ids.h +1 -0
@@ -914,6 +914,7 @@#define USB_DEVICE_ID_PICK16F1454_V2 0xf2f7#define USB_DEVICE_ID_LUXAFOR 0xf372#define USB_DEVICE_ID_MCP2221 0x00dd +#define USB_DEVICE_ID_MCP2200 0x00df#define USB_VENDOR_ID_MICROSOFT 0x045e#define USB_DEVICE_ID_SIDEWINDER_GV 0x003b
drivers/hid/hid-mcp2200.c new file +419 -0
419 lines not on this page · raw message
Signature · 1 line
2.41.0
Commit message subject should probably start with "HID: mcp2200:" rather than "hid-mcp2200:". This matches mcp2221.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/hid/hid-mcp2221.c
Quoted 9 lines · On Fri, 18 Aug, 2023 12:48:52 +0200 <[email protected]> wrote:
From: Johannes Roith <[email protected]>
Added a gpiochip compatible driver to control the 8 GPIOs of the MCP2200 by using the HID interface.
Using GPIOs with alternative functions (GP0<->SSPND, GP1<->USBCFG, GP6<->RXLED, GP7<->TXLED) will reset the functions, if set (unset by default).
Quoted 7 lines
The driver was tested while also using the UART of the chip. Setting and reading the GPIOs has no effect on the UART communication. However, a reset is triggered after the CONFIGURE command. If the GPIO Direction is constantly changed, this will affect the communication at low baud rates. This is a hardware problem of the MCP2200 and is not caused by the driver.
The content below should not be in the commit message body but rather git notes or a section in the patch that will not be applied.
Quoted 11 lines
Feedback from reviewers Christophe JAILLET <[email protected]> and Andi Shyti <[email protected]> was added.
Changelog:
- v1: added driver
- v2: added ProductID inhid-ids.h
- v3: added feedback from review and make patch compilable
- v4: added more feedback from second review, tested as module and builtin on x86 and arm64
- v5: adoptedKconfigto style guidelines, fixedmcp_set_direction
For your reference in case you find it helpful.
* https://git-scm.com/docs/git-notes
* https://git-scm.com/docs/git-format-patch#Documentation/git-format-patch.txt---notesltrefgt
Quoted 18 lines
Signed-off-by Johannes Roith <[email protected]>--- drivers/hid/Kconfig | 10 + drivers/hid/Makefile | 1 + drivers/hid/hid-ids.h | 1 + drivers/hid/hid-mcp2200.c | 419 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 431 insertions(+) create mode 100644 drivers/hid/hid-mcp2200.cdrivers/hid/Kconfig +0
-0@@ -1301,6 +1301,16 @@ config HID_MCP2221 To compile this driver as a module, choose M here: the module will be called hid-mcp2221.ko.
drivers/hid/Kconfig config HID_MCP2221 · line 1313
Quoted 10 lines
drivers/hid/Kconfig +10
-0+config HID_MCP2200 + tristate "Microchip MCP2200 HID USB-to-GPIO bridge" + depends on USB_HID + imply GPIOLIB + help + Provides GPIO functionality over USB-HID through MCP2200 device. + + To compile this driver as a module, choose M here: the module + will be called hid-mcp2200.ko. +
Alphabetically, HID_MCP2200 should be before HID_MCP2221.
drivers/hid/Makefile obj-$(CONFIG_HID_MACALLY) += hid-macally.o · line 82
Quoted 12 lines
drivers/hid/Kconfig +0
-0config HID_KUNIT_TEST tristate "KUnit tests for HID" if !KUNIT_ALL_TESTS depends on KUNITdrivers/hid/Makefile +1
-0@@ -79,6 +79,7 @@ obj-$(CONFIG_HID_MACALLY) += hid-macally.o obj-$(CONFIG_HID_MAGICMOUSE) += hid-magicmouse.o obj-$(CONFIG_HID_MALTRON) += hid-maltron.o obj-$(CONFIG_HID_MCP2221) += hid-mcp2221.o +obj-$(CONFIG_HID_MCP2200) += hid-mcp2200.o
Similar comment as previous here.
drivers/hid/hid-ids.h line 917
Quoted 12 lines
drivers/hid/Makefile +0
-0obj-$(CONFIG_HID_MAYFLASH) += hid-mf.o obj-$(CONFIG_HID_MEGAWORLD_FF) += hid-megaworld.o obj-$(CONFIG_HID_MICROSOFT) += hid-microsoft.odrivers/hid/hid-ids.h +1
-0@@ -914,6 +914,7 @@#define USB_DEVICE_ID_PICK16F1454_V2 0xf2f7#define USB_DEVICE_ID_LUXAFOR 0xf372#define USB_DEVICE_ID_MCP2221 0x00dd +#define USB_DEVICE_ID_MCP2200 0x00df
This file is sorted primarily by the define name rather than the device id, so I would expect the define for mcp2200 to be before mcp2221 (even though its device id would be after).
Quoted 108 lines
drivers/hid/hid-ids.h +0
-0#define USB_VENDOR_ID_MICROSOFT 0x045e#define USB_DEVICE_ID_SIDEWINDER_GV 0x003bdrivers/hid/hid-mcp2200.c new file +99
-0@@ -0,0 +1,419 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* +* MCP2200 - Microchip USB to GPIO bridge +* +* Copyright (c) 2023, Johannes Roith <[email protected]> +* +* Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/22228A.pdf +* App Note for HID: https://ww1.microchip.com/downloads/en/DeviceDoc/93066A.pdf +*/ +#include <linux/completion.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/gpio/driver.h> +#include <linux/hid.h> +#include <linux/hidraw.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include "hid-ids.h" + +/* Commands codes in a raw output report */ +#define SET_CLEAR_OUTPUTS 0x08 +#define CONFIGURE 0x10 +#define READ_EE 0x20 +#define WRITE_EE 0x40 +#define READ_ALL 0x80 + +/* MCP GPIO direction encoding */ +enum MCP_IO_DIR { +MCP2200_DIR_OUT =0x00 , +MCP2200_DIR_IN =0x01 , +}; + +/* Altternative pin assignments */ +#define TXLED 2 +#define RXLED 3 +#define USBCFG 6 +#define SSPND 7 +#define MCP_NGPIO 8 + +/* CMD to set or clear a GPIO output */ +struct mcp_set_clear_outputs { +u8 cmd; +u8 dummys1[10 ]; +u8 set_bmap; +u8 clear_bmap; +u8 dummys2[3 ]; +} __packed; + +/* CMD to configure the IOs */ +struct mcp_configure { +u8 cmd; +u8 dummys1[3 ]; +u8 io_bmap; +u8 config_alt_pins; +u8 io_default_val_bmap; +u8 config_alt_options; +u8 baud_h; +u8 baud_l; +u8 dummys2[6 ]; +} __packed; + +/* CMD to read all parameters */ +struct mcp_read_all { +u8 cmd; +u8 dummys[15 ]; +} __packed; + +/* Response to the read all cmd */ +struct mcp_read_all_resp { +u8 cmd; +u8 eep_addr; +u8 dummy; +u8 eep_val; +u8 io_bmap; +u8 config_alt_pins; +u8 io_default_val_bmap; +u8 config_alt_options; +u8 baud_h; +u8 baud_l; +u8 io_port_val_bmap; +u8 dummys[5 ]; +} __packed; + +struct mcp2200 { +struct hid_device *hdev; +struct mutex lock; +struct completion wait_in_report; +u8 gpio_dir; +u8 gpio_val; +u8 gpio_inval; +u8 baud_h; +u8 baud_l; +u8 config_alt_pins; +u8 gpio_reset_val; +u8 config_alt_options; +int status; +struct gpio_chip gc; +}; +
drivers/hid/hid-mcp2200.c line 109
Quoted 10 lines
drivers/hid/hid-mcp2200.c new file +10
-0+/* this executes the READ_ALL cmd */ +static int mcp_cmd_read_all (struct mcp2200 *mcp) +{ +struct mcp_read_all *read_all; +int len, t; + +reinit_completion (&mcp->wait_in_report); + read_all =kzalloc (sizeof (struct mcp_read_all ),GFP_KERNEL ); +if (!read_all) +return -ENOMEM ;
My personal recommendation is to just have a single DMA buffer allocated for the mcp2200 instance rather than having to call the allocator and release the memory per command.
Here is a reference.
Quoted 60 lines
drivers/hid/hid-mcp2200.c new file +60
-0+ + read_all->cmd =READ_ALL ; + +mutex_lock (&mcp->lock); + len =hid_hw_output_report (mcp->hdev, (u8 *) read_all, +sizeof (struct mcp_read_all )); + +mutex_unlock (&mcp->lock); +kfree (read_all); + +if (len !=sizeof (struct mcp_read_all )) +return -EINVAL ; + + t =wait_for_completion_timeout (&mcp->wait_in_report, +msecs_to_jiffies (4000 )); +if (!t) +return -ETIMEDOUT ; + +/* return status, negative value if wrong response was received */ +return mcp->status; +} + +static void mcp_set_multiple (struct gpio_chip *gc,unsigned long *mask, +unsigned long *bits) +{ +struct mcp2200 *mcp =gpiochip_get_data (gc); +u8 value; +int status; +struct mcp_set_clear_outputs *cmd; + + cmd =kzalloc (sizeof (struct mcp_set_clear_outputs ),GFP_KERNEL ); +if (!cmd) +return ; + +mutex_lock (&mcp->lock); + + value = mcp->gpio_val & ~*mask; + value |= (*mask & *bits); + + cmd->cmd =SET_CLEAR_OUTPUTS ; + cmd->set_bmap = value; + cmd->clear_bmap = ~(value); + + status =hid_hw_output_report (mcp->hdev, (u8 *) cmd, +sizeof (struct mcp_set_clear_outputs )); + +if (status ==sizeof (struct mcp_set_clear_outputs )) + mcp->gpio_val = value; + +mutex_unlock (&mcp->lock); +kfree (cmd); +} + +static void mcp_set (struct gpio_chip *gc,unsigned int gpio_nr,int value) +{ +unsigned long mask =1 << gpio_nr; +unsigned long bmap_value = value << gpio_nr; + +mcp_set_multiple (gc, &mask, &bmap_value); +}
drivers/hid/hid-mcp2200.c line 179
Quoted 10 lines
drivers/hid/hid-mcp2200.c new file +10
-0+ +static int mcp_get_multiple (struct gpio_chip *gc,unsigned long *mask, +unsigned long *bits) +{ +u32 val; +struct mcp2200 *mcp =gpiochip_get_data (gc); +int status; + + status =mcp_cmd_read_all (mcp); +if (status !=0 )
Can be simplified as 'if (status)'.
drivers/hid/hid-mcp2200.c line 189
Quoted 10 lines
drivers/hid/hid-mcp2200.c new file +10
-0+return status; + + val = mcp->gpio_inval; + *bits = (val & *mask); +return 0 ; +} + +static int mcp_get (struct gpio_chip *gc,unsigned int gpio_nr) +{ +unsigned long mask =0 , bits =0 ;
You do not need to zero init these variables? It's useful to avoid initializing variables unnecessarily for compile time checking/static analysis purposes.
Quoted 78 lines
drivers/hid/hid-mcp2200.c new file +78
-0+ + mask = (1 << gpio_nr); +mcp_get_multiple (gc, &mask, &bits); +return bits >0 ; +} + +static int mcp_get_direction (struct gpio_chip *gc,unsigned int gpio_nr) +{ +struct mcp2200 *mcp =gpiochip_get_data (gc); + +return (mcp->gpio_dir & (MCP2200_DIR_IN << gpio_nr)) + ?GPIO_LINE_DIRECTION_IN :GPIO_LINE_DIRECTION_OUT ; +} + +static int mcp_set_direction (struct gpio_chip *gc,unsigned int gpio_nr, +enum MCP_IO_DIR io_direction) +{ +struct mcp2200 *mcp =gpiochip_get_data (gc); +struct mcp_configure *conf; +int status; +/* after the configure cmd we will need to set the outputs again */ +unsigned long mask = ~(mcp->gpio_dir);/* only set outputs */ +unsigned long bits = mcp->gpio_val; +/* Offsets of alternative pins in config_alt_pins, 0 is not used */ +u8 alt_pin_conf[8 ] = {SSPND ,USBCFG ,0 ,0 ,0 ,0 ,RXLED ,TXLED }; +u8 config_alt_pins = mcp->config_alt_pins; + +/* Read in the reset baudrate first, we need it later */ + status =mcp_cmd_read_all (mcp); +if (status !=0 ) +return status; + + conf =kzalloc (sizeof (struct mcp_configure ),GFP_KERNEL ); +if (!conf) +return -ENOMEM ; + +mutex_lock (&mcp->lock); + +/* configure will reset the chip! */ + conf->cmd =CONFIGURE ; + conf->io_bmap = (mcp->gpio_dir & ~(1 << gpio_nr)) + | (io_direction << gpio_nr); +/* Don't overwrite the reset parameters */ + conf->baud_h = mcp->baud_h; + conf->baud_l = mcp->baud_l; + conf->config_alt_options = mcp->config_alt_options; + conf->io_default_val_bmap = mcp->gpio_reset_val; +/* Adjust alt. func if necessary */ +if (alt_pin_conf[gpio_nr]) + config_alt_pins &= ~(1 << alt_pin_conf[gpio_nr]); + conf->config_alt_pins = config_alt_pins; + + status =hid_hw_output_report (mcp->hdev, (u8 *) conf, +sizeof (struct mcp_set_clear_outputs )); + +if (status ==sizeof (struct mcp_set_clear_outputs )) { + mcp->gpio_dir = conf->io_bmap; + mcp->config_alt_pins = config_alt_pins; + }else { +mutex_unlock (&mcp->lock); +kfree (conf); +return -EIO ; + } + +mutex_unlock (&mcp->lock); +kfree (conf); + +/* Configure CMD will clear all IOs -> rewrite them */ +mcp_set_multiple (gc, &mask, &bits); +return 0 ; +} + +static int mcp_direction_input (struct gpio_chip *gc,unsigned int gpio_nr) +{ +return mcp_set_direction (gc, gpio_nr,MCP2200_DIR_IN ); +} + +static int mcp_direction_output(struct gpio_chip *gc, unsigned int gpio_nr,
drivers/hid/hid-mcp2200.c line 277
Quoted 10 lines
drivers/hid/hid-mcp2200.c new file +10
-0+ int value) +{ +int ret; +unsigned long mask, bmap_value; + + mask =1 << gpio_nr; + bmap_value = value << gpio_nr; + + ret =mcp_set_direction (gc, gpio_nr,MCP2200_DIR_OUT ); +if (ret ==0 )
'if (!ret)' can be used here.
Quoted 38 lines
drivers/hid/hid-mcp2200.c new file +38
-0+mcp_set_multiple (gc, &mask, &bmap_value); +return ret; +} + +static const struct gpio_chip template_chip = { + .label ="mcp2200" , + .owner =THIS_MODULE , + .get_direction = mcp_get_direction, + .direction_input = mcp_direction_input, + .direction_output = mcp_direction_output, + .set = mcp_set, + .set_multiple = mcp_set_multiple, + .get = mcp_get, + .get_multiple = mcp_get_multiple, + .base =-1 , + .ngpio =MCP_NGPIO , + .can_sleep = true, +}; + +/* +* MCP2200 uses interrupt endpoint for input reports. This function +* is called by HID layer when it receives i/p report from mcp2200, +* which is actually a response to the previously sent command. +*/ +static int mcp2200_raw_event (struct hid_device *hdev,struct hid_report *report, +u8 *data,int size) +{ +struct mcp2200 *mcp =hid_get_drvdata (hdev); +struct mcp_read_all_resp *all_resp; + +switch (data[0 ]) { +case READ_ALL : + all_resp = (struct mcp_read_all_resp *) data; + mcp->status =0 ; + mcp->gpio_inval = all_resp->io_port_val_bmap; + mcp->baud_h = all_resp->baud_h; + mcp->baud_l = all_resp->baud_l; + mcp->gpio_reset_val = all_resp->io_default_val_bmap;
drivers/hid/hid-mcp2200.c line 325
Quoted 10 lines
drivers/hid/hid-mcp2200.c new file +10
-0+ mcp->config_alt_pins = all_resp->config_alt_pins; + mcp->config_alt_options = all_resp->config_alt_options; +break ; +default : + mcp->status = -EIO ; +break ; + } + +complete (&mcp->wait_in_report); +return 1 ;
While this is not problematic today, I think it would be better to return 0 here rather than 1.
Quoted 30 lines
drivers/hid/hid-mcp2200.c new file +30
-0+} + +static void mcp2200_hid_unregister (void *ptr) +{ +struct hid_device *hdev = ptr; + +hid_hw_close (hdev); +hid_hw_stop (hdev); +} + +static int mcp2200_probe (struct hid_device *hdev,const struct hid_device_id *id) +{ +int ret; +struct mcp2200 *mcp; + + mcp =devm_kzalloc (&hdev->dev,sizeof (*mcp),GFP_KERNEL ); +if (!mcp) +return -ENOMEM ; + + ret =hid_parse (hdev); +if (ret) { +hid_err (hdev,"can't parse reports\n" ); +return ret; + } + +/* +* This driver uses the .raw_event callback and therefore does not need any +* HID_CONNECT_xxx flags. +*/ + ret =hid_hw_start (hdev,0 );
drivers/hid/hid-mcp2200.c line 365
Quoted 10 lines
drivers/hid/hid-mcp2200.c new file +10
-0+if (ret) { +hid_err (hdev,"can't start hardware\n" ); +return ret; + } + +hid_info (hdev,"USB HID v%x.%02x Device [%s] on %s\n" , hdev->version >>8 , + hdev->version &0xff , hdev->name, hdev->phys); + + ret =hid_hw_open (hdev); +if (ret) {
Need to call hid_hw_stop here since hid_hw_start succeeded.
drivers/hid/hid-mcp2200.c line 376
Quoted 11 lines
drivers/hid/hid-mcp2200.c new file +11
-0+hid_err (hdev,"can't open device\n" ); +return ret; + } + +mutex_init (&mcp->lock); +init_completion (&mcp->wait_in_report); +hid_set_drvdata (hdev, mcp); + mcp->hdev = hdev; + + ret =devm_add_action_or_reset (&hdev->dev, mcp2200_hid_unregister, hdev); +if (ret)
You probably want to call mcp2200_hid_unregister here since the action was not added?
Quoted 11 lines
drivers/hid/hid-mcp2200.c new file +11
-0+return ret; + + mcp->gc = template_chip; + mcp->gc.parent = &hdev->dev; + + ret =devm_gpiochip_add_data (&hdev->dev, &mcp->gc, mcp); +if (ret <0 ) { +hid_err (hdev,"Unable to register gpiochip\n" ); +return ret; + } +
drivers/hid/hid-mcp2200.c line 397
Quoted 10 lines
drivers/hid/hid-mcp2200.c new file +10
-0+return 0 ; +} + +static void mcp2200_remove (struct hid_device *hdev) +{ + (void ) hdev; +/* +* With no remove function you sometimes get a segmentation fault when +* unloading the module or disconnecting the USB device +*/
The reason you run into this is likely because of the action added to devm conflicting with hid_device_remove....
I recommend not depending on devm for teardown rather than making a stub remove function to work around the issue.
static void hid_device_remove(struct device *dev)
{
struct hid_device *hdev = to_hid_device(dev);
struct hid_driver *hdrv; down(&hdev->driver_input_lock);
hdev->io_started = false;
hdrv = hdev->driver;
if (hdrv) {
if (hdrv->remove)
hdrv->remove(hdev);
else /* default remove */
hid_hw_stop(hdev);hid_device_remove will call hid_hw_stop and so will mcp2200_hid_unregister because of the devm action you added.
Quoted 12 lines
drivers/hid/hid-mcp2200.c new file +12
-0+} + +static const struct hid_device_id mcp2200_devices[] = { + {HID_USB_DEVICE (USB_VENDOR_ID_MICROCHIP ,USB_DEVICE_ID_MCP2200 ) }, + { } +}; +MODULE_DEVICE_TABLE (hid, mcp2200_devices); + +static struct hid_driver mcp2200_driver = { + .name ="mcp2200" , + .id_table = mcp2200_devices, + .probe = mcp2200_probe,
drivers/hid/hid-mcp2200.c line 419
Quoted 10 lines
drivers/hid/hid-mcp2200.c new file +10
-0+ .remove = mcp2200_remove, + .raw_event = mcp2200_raw_event, +}; + +/* Register with HID core */ +module_hid_driver (mcp2200_driver); + +MODULE_AUTHOR ("Johannes Roith <[email protected]>" ); +MODULE_DESCRIPTION ("MCP2200 Microchip HID USB to GPIO bridge" ); +MODULE_LICENSE ("GPL" );
--
Thanks,
Rahul Rameshbabu
Hi Rahul,
thanks for the feedback, I will add it to the driver.
Quoted 3 lines
My personal recommendation is to just have a single DMA buffer allocated for the mcp2200 instance rather than having to call the allocator and release the memory per command.
I added an 16-byte Array hid_report to the mcp2000 struct. When I need the report, I do the following:
struct mcp_set_clear_outputs *cmd;
mutex_lock(&mcp->lock);
cmd = (struct mcp_set_clear_outputs *) mcp->hid_report
Do you think this is a valid implementation or do I really have to add a pointer to the mcp2200 struct instead of the 16 byte array and allocate another 16 byte for it in the probe function?
Quoted 5 lines
The reason you run into this is likely because of the action added to devm conflicting with
hid_device_remove....I recommend not depending on devm for teardown rather than making a stub remove function to work around the issue.
I am not sure, if I have understand this correctly, but basically I already have a stub remove function (which is empty). First the remove function gets called, then the unregister function and everything is cleaned up correctly. Did I get this right or do you have any other recommendation for me?
So, do I need any adaptions, or can we go with the empty remove function?
Best regards,
Johannes