Re: [char-misc-next] mei: gsc_proxy: add gsc proxy driver
From: Greg Kroah-Hartman
Date: Fri Dec 23 2022 - 02:17:46 EST
On Fri, Dec 23, 2022 at 12:02:14AM +0200, Tomas Winkler wrote:
> From: Alexander Usyskin <alexander.usyskin@xxxxxxxxx>
>
> Add GSC proxy driver. It to allows messaging between GSC component
> on Intel on board graphics card and CSME device.
> GSC and MEI
>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@xxxxxxxxx>
> Signed-off-by: Tomas Winkler <tomas.winkler@xxxxxxxxx>
> ---
> MAINTAINERS | 2 +-
> drivers/misc/mei/Kconfig | 1 +
> drivers/misc/mei/Makefile | 1 +
> drivers/misc/mei/gsc_proxy/Kconfig | 13 ++
> drivers/misc/mei/gsc_proxy/Makefile | 7 +
> drivers/misc/mei/gsc_proxy/mei_gsc_proxy.c | 205 +++++++++++++++++++++
Why a whole new subdirectory for a tiny 200 line file?
> +static int mei_gsc_proxy_component_match(struct device *dev, int subcomponent, void *data)
> +{
> + struct device *base = data;
> +
> + if (!dev || !dev->driver ||
> + strcmp(dev->driver->name, "i915") ||
I thought I had objected to this "let's poke around in a driver name for
a magic value" logic in the past. How do you know this is always going
to work?
> + subcomponent != I915_COMPONENT_GSC_PROXY)
> + return 0;
> +
> + base = base->parent;
> + if (!base) /* mei device */
> + return 0;
How can a device not have a parent?
> +
> + base = base->parent; /* pci device */
You don't know this is a pci device :(
If it is, then pass in a REAL pci device structure please.
> +
> + dev = dev->parent;
> + return (base && dev && dev == base);
I do not understand this statement at all, what are you doing here?
confused,
greg k-h