Re: [PATCH v2 5/5] misc: add ge-addon-connector driver
From: Greg Kroah-Hartman
Date: Fri May 10 2024 - 03:55:42 EST
On Fri, May 10, 2024 at 09:10:41AM +0200, Luca Ceresoli wrote:
> Add a driver to support the runtime hot-pluggable add-on connector on the
> GE SUNH device. This connector allows connecting and disconnecting an
> add-on to/from the main device to augment its features. Connection and
> disconnection can happen at runtime at any moment without notice.
>
> Different add-on models can be connected, and each has an EEPROM with a
> model identifier at a fixed address.
>
> The add-on hardware is added and removed using device tree overlay loading
> and unloading.
>
> Co-developed-by: Herve Codina <herve.codina@xxxxxxxxxxx>
> Signed-off-by: Herve Codina <herve.codina@xxxxxxxxxxx>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>
>
> ---
>
> This commit is new in v2.
> ---
> MAINTAINERS | 1 +
> drivers/misc/Kconfig | 15 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/ge-sunh-connector.c | 464 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 481 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 672c26372c92..0bdb4fc496b8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9905,6 +9905,7 @@ F: drivers/iio/pressure/mprls0025pa*
> HOTPLUG CONNECTOR FOR GE SUNH ADDONS
> M: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>
> S: Maintained
> +F: drivers/misc/ge-sunh-connector.c
> F: Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
>
> HP BIOSCFG DRIVER
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 4fb291f0bf7c..99ef2eccbbaa 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -574,6 +574,21 @@ config NSM
> To compile this driver as a module, choose M here.
> The module will be called nsm.
>
> +config GE_SUNH_CONNECTOR
> + tristate "GE SUNH hotplug add-on connector"
> + depends on OF
> + select OF_OVERLAY
> + select FW_LOADER
> + select NVMEM
> + select DRM_HOTPLUG_BRIDGE
Can these be depends instead of select? 'select' causes dependencies
that are hard, if not almost impossible, to detect at times why
something is being enabled.
> + help
> + Driver for the runtime hot-pluggable add-on connector on the GE SUNH
> + device. This connector allows connecting and disconnecting an add-on
> + to/from the main device to augment its features. Connection and
> + disconnection can be done at runtime at any moment without
> + notice. Different add-on models can be connected, and each has an EEPROM
> + with a model identifier at a fixed address.
Module name?
> +static void sunh_conn_reset(struct sunh_conn *conn, bool keep_reset)
> +{
> + dev_dbg(conn->dev, "reset\n");
ftrace is your friend.
> +static int sunh_conn_handle_event(struct sunh_conn *conn, bool plugged)
> +{
> + int err;
> +
> + if (plugged == conn->plugged)
> + return 0;
> +
> + dev_info(conn->dev, "%s\n", plugged ? "connected" : "disconnected");
Please remove debugging code from stuff you want to see merged.
Same for all dev_info() calls here, when drivers work properly, they are
quiet.
thanks,
greg k-h