On Tue, 18 Apr 2023, "Teres Alexis, Alan Previn" <alan.previn.teres.alexis@xxxxxxxxx> wrote: > On Wed, 2023-03-29 at 09:56 -0700, Ceraolo Spurio, Daniele wrote: >> From: Alexander Usyskin <alexander.usyskin@xxxxxxxxx> >> >> GSC Proxy component is used for communication between the >> Intel graphics driver and MEI driver. > > > >> diff --git a/include/drm/i915_gsc_proxy_mei_interface.h b/include/drm/i915_gsc_proxy_mei_interface.h >> new file mode 100644 >> index 000000000000..e817bb316d5c >> --- /dev/null >> +++ b/include/drm/i915_gsc_proxy_mei_interface.h >> @@ -0,0 +1,36 @@ >> +/* SPDX-License-Identifier: MIT */ >> +/* >> + * Copyright (c) 2022-2023 Intel Corporation >> + */ >> + >> +#ifndef _I915_GSC_PROXY_MEI_INTERFACE_H_ >> +#define _I915_GSC_PROXY_MEI_INTERFACE_H_ >> + >> +#include <linux/mutex.h> > > alan: i notice u have included mutex.h but don't have any mutex use in this header. > side note: looking at at least one other component interfaces (pxp), I see a mutex in the > component struct but don't see it being used at all - a topic for a different series. > > >> +#include <linux/device.h> > alan: any reason we should include "device.h"? as opposed to just define the ptr type > (since w only reference the ptrs). this could save us a little on build time The only thing required is <linux/types.h>. Everything else can be forward declared. BR, Jani. > >> + >> +/** >> + * struct i915_gsc_proxy_component_ops - ops for GSC Proxy services. >> + * @owner: Module providing the ops >> + * @send: sends data through GSC proxy >> + * @recv: receives data through GSC proxy > alan: nit: to be more specific "... from i915 through GSC proxy" > >> + */ >> +struct i915_gsc_proxy_component_ops { >> + struct module *owner; >> + >> + int (*send)(struct device *dev, const void *buf, size_t size); >> + int (*recv)(struct device *dev, void *buf, size_t size); >> +}; > alan: i believe we should have proper documentation on the possible list of > return values for the various error conditions (assuming 0 is success). > >> + >> +/** >> + * struct i915_gsc_proxy_component - Used for communication between i915 and >> + * MEI drivers for GSC proxy services >> + * @mei_dev: device that provide the GSC proxy service. >> + * @ops: Ops implemented by GSC proxy driver, used by i915 driver. >> + */ >> +struct i915_gsc_proxy_component { >> + struct device *mei_dev; >> + const struct i915_gsc_proxy_component_ops *ops; >> +}; >> + >> +#endif /* _I915_GSC_PROXY_MEI_INTERFACE_H_ */ > -- Jani Nikula, Intel Open Source Graphics Center