public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Iouri Tarassov <iourit@linux.microsoft.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Sasha Levin <sashal@kernel.org>,
	kys@microsoft.com, haiyangz@microsoft.com,
	sthemmin@microsoft.com, wei.liu@kernel.org, iourit@microsoft.com,
	linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
	spronovo@microsoft.com
Subject: Re: [PATCH 1/4] drivers: hv: dxgkrnl: core code
Date: Thu, 3 Sep 2020 11:55:16 -0700	[thread overview]
Message-ID: <d8f6ed37-11dc-1103-8908-ad79482a4694@linux.microsoft.com> (raw)
In-Reply-To: <20200828061257.GB56396@kroah.com>

Hi Greg,

I appreciate your comments and working to address them.

On 8/27/2020 11:12 PM, Greg KH wrote:
> As for "matching names", why does that matter?  Who sees both names at
> the same time?
>
> > > 
> > > endian issues?
> > > 
> > > If not, why are these bit fields?
> > This matches the definition on the Windows side. Windows only works on
> > little endian platforms.
>
> But Linux works on both, so you need to properly document/handle this somehow.
This driver works only in a Linux container in conjunction with the 
Windows host. The structure definitions are  the same on the host and 
the container. The driver will not be enabled or work on platforms, 
where Windows does not run.
>
> > > 
> > > > +struct d3dkmt_destroydevice {
> > > > +	struct d3dkmthandle		device;
> > > > +};
> > > 
> > > Again, single entity structures?
> > > 
> > > Are you trying to pass around "handles" and cast them backwards?
> > > 
> > > If so, great, but then use the real kernel structures for that like
> > > 'struct device' if these are actually devices.
> > > 
> > Again. The structure matches the definition on the Windows side to avoid
> > confusion.
>
> Who is confused here?  We accept naming conventions that do not match
> the normal Linux style when they are referring to external sources of
> the data.  Examples of this are USB device field names, and other
> hardware specifications that are public.  You aren't sharing code with a
> Windows system, so please follow the Linux coding style rules, as you
> want Linux developers to be helping you maintain this code, not
> developers who have ever read code from other operating systems.
>
> So please follow the rule of, "unless these fields and structures are
> publically defined somewhere, use Linux naming rules", like all of the
> rest of us do.
>
The d3dkmt* structures, like d3dkmt_destroydevice are publicly 
documented on MSDN 
(https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/d3dkmthk/ns-d3dkmthk-_d3dkmt_destroydevice). 
I am using the same definitions in the driver, so it is easy to find the 
corresponding definition and description of the structure. I have no 
problem to change the names, but I think using the same public 
definition will help the driver maintainers.

Thanks

Iouri


  reply	other threads:[~2020-09-03 18:55 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-14 12:38 [PATCH 0/4] drivers: hv: Microsoft Virtual GPU Driver Sasha Levin
2020-08-14 12:38 ` [PATCH 1/4] drivers: hv: dxgkrnl: core code Sasha Levin
2020-08-14 12:55   ` Greg KH
2020-08-27 23:29     ` Iouri Tarassov
2020-08-28  6:01       ` Greg KH
2020-08-14 12:57   ` Greg KH
2020-08-27 23:45     ` Iouri Tarassov
2020-08-28  6:15       ` Greg KH
2020-08-14 13:04   ` Greg KH
2020-08-28  0:05     ` Iouri Tarassov
2020-08-28  6:12       ` Greg KH
2020-09-03 18:55         ` Iouri Tarassov [this message]
2020-09-03 19:32           ` Greg KH
2020-08-14 13:18   ` Wei Liu
2020-08-26 20:20     ` Iouri Tarassov
2020-08-27  0:12     ` Iouri Tarassov
2020-08-27 19:09     ` Iouri Tarassov
2020-08-21 13:53   ` Pavel Machek
2020-08-28  0:25     ` Iouri Tarassov
2020-08-28  6:17       ` Greg KH
2020-08-28  6:18       ` Greg KH
2020-09-03 21:39         ` Iouri Tarassov
2020-09-04  5:18           ` Greg KH
2020-08-14 12:38 ` [PATCH 2/4] drivers: hv: dxgkrnl: hook up dxgkrnl Sasha Levin
2020-08-14 12:38 ` [PATCH 3/4] drivers: hv: vmbus: " Sasha Levin
2020-08-14 12:38 ` [PATCH 4/4] drivers: hv: dxgkrnl: create a MAINTAINERS entry Sasha Levin
2020-08-14 13:04   ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d8f6ed37-11dc-1103-8908-ad79482a4694@linux.microsoft.com \
    --to=iourit@linux.microsoft.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=haiyangz@microsoft.com \
    --cc=iourit@microsoft.com \
    --cc=kys@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sashal@kernel.org \
    --cc=spronovo@microsoft.com \
    --cc=sthemmin@microsoft.com \
    --cc=wei.liu@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox