i guess we are settled with this patch...
On Thu, 2023-04-20 at 15:04 -0700, Ceraolo Spurio, Daniele wrote:
> On 4/18/2023 11:57 PM, Teres Alexis, Alan Previn wrote:
> > On Wed, 2023-03-29 at 09:56 -0700, Ceraolo Spurio, Daniele 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 CSE device.
> > alan:nit: isn't "Intel integrated GPU" clearer than "Intel on-board graphics card"?
> > Same thing for the Kconfig description later (or am i missing something else here).
>
> Will change
>
alan: saw your reply on better alternative for both 'i' and 'd'
alan:snip
> > > +static int mei_gsc_proxy_recv(struct device *dev, void *buf, size_t size)
> > > +{
> > > + ssize_t ret;
> > > +
> > > + if (!dev || !buf)
> > alan: nit: same as in the 'send' above,.. not sure if we should be checking for !size here...
> > or perhaps 0 sized recv is supported.
>
> AFAICS the lower level of the mei code do allow for size 0 for both send
> and recv. Also, this is the same check as what we do for the PXP component.
alan: agreed - thus the nit as per my earlier email.
> >
alan:snip
> > > + if (subcomponent != I915_COMPONENT_GSC_PROXY)
> > > + return 0;
> > > +
> > > + return component_compare_dev(dev->parent, ((struct device *)data)->parent);
> > alan: do we care if both these parents are non-null? i notice in other mei component
> > drivers match functions we do check that.
>
> Those should always both be non-NULL, since both the mei and the GFX
> device have the PCI bus as parent (and the previous check on pdev
> ensures those are the 2 devices we're handling at this point).
alan: sounds good.
> > > +#define MEI_UUID_GSC_PROXY UUID_LE(0xf73db04, 0x97ab, 0x4125, \
> > > + 0xb8, 0x93, 0xe9, 0x4, 0xad, 0xd, 0x54, 0x64)
> > alan: apologies for the newbie question, but why are we using UUID for the gsc_proxy
> > as opposed to GUID like the other mei components? i am not sure if i read the right
> > archived patch review but it sounded like GUID is for internal to kernel only whereas
> > UUID is for external too?
[snip]
> AFAICS all other mei components use UUID_LE as well. The code was
> updated from GUID to UUID_LE in:
> https://lore.kernel.org/all/20221228160558.21311-1-andriy.shevchenko@xxxxxxxxxxxxxxx/
alan: sounds good- thanks for the URL.
