Linux-WPAN Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH] Cleanup: replace prefered with preferred
       [not found] <20191022214208.211448-1-salyzyn@android.com>
@ 2019-10-23 10:22 ` Laurent Pinchart
  2019-10-23 11:56 ` Jarkko Sakkinen
  1 sibling, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2019-10-23 10:22 UTC (permalink / raw)
  To: Mark Salyzyn
  Cc: linux-kernel, kernel-team, David S. Miller, Jonathan Corbet,
	Ard Biesheuvel, Harry Wentland, Leo Li, Alex Deucher,
	Christian König, David (ChunMing) Zhou, David Airlie,
	Daniel Vetter, VMware Graphics, Thomas Hellstrom,
	Mauro Carvalho Chehab, Trond Myklebust, Anna Schumaker,
	Alexander Aring, Jukka Rissanen, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, Jarkko Sakkinen, Ingo Molnar, Matthew Garrett,
	Hans de Goede, hersen wu, Roman Li, Maxim Martynov, David Ahern,
	Francesco Ruggeri, Linus Lüssing, Greg Kroah-Hartman,
	Feng Tang, Steven Rostedt (VMware),
	Andrew Morton, Rafael Aquini, netdev, linux-doc, linux-efi,
	amd-gfx, dri-devel, linux-media, linux-nfs, linux-bluetooth,
	linux-wpan

Hi Mark,

Thank you for the patch.

On Tue, Oct 22, 2019 at 02:41:45PM -0700, Mark Salyzyn wrote:
> Replace all occurrences of prefered with preferred to make future
> checkpatch.pl's happy.  A few places the incorrect spelling is
> matched with the correct spelling to preserve existing user space API.
> 
> Signed-off-by: Mark Salyzyn <salyzyn@android.com>
> ---
>  Documentation/networking/ip-sysctl.txt        |   2 +-
>  .../firmware/efi/libstub/efi-stub-helper.c    |   2 +-
>  .../gpu/drm/amd/display/dc/inc/compressor.h   |   4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |   2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h           |   2 +-
>  drivers/media/usb/uvc/uvc_video.c             |   6 +-
>  fs/nfs/nfs4xdr.c                              |   2 +-
>  include/linux/ipv6.h                          |   2 +-
>  include/net/addrconf.h                        |   4 +-
>  include/net/if_inet6.h                        |   2 +-
>  include/net/ndisc.h                           |   8 +-
>  include/uapi/linux/if_addr.h                  |   5 +-
>  include/uapi/linux/ipv6.h                     |   4 +-
>  include/uapi/linux/sysctl.h                   |   4 +-
>  include/uapi/linux/usb/video.h                |   5 +-
>  kernel/sysctl_binary.c                        |   3 +-
>  net/6lowpan/ndisc.c                           |   4 +-
>  net/ipv4/devinet.c                            |  20 ++--
>  net/ipv6/addrconf.c                           | 113 ++++++++++--------
>  19 files changed, 112 insertions(+), 82 deletions(-)

[snip]

> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 8fa77a81dd7f..0096e6aacdb4 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -276,13 +276,13 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
>  	if (size >= 34) {
>  		ctrl->dwClockFrequency = get_unaligned_le32(&data[26]);
>  		ctrl->bmFramingInfo = data[30];
> -		ctrl->bPreferedVersion = data[31];
> +		ctrl->bPreferredVersion = data[31];
>  		ctrl->bMinVersion = data[32];
>  		ctrl->bMaxVersion = data[33];
>  	} else {
>  		ctrl->dwClockFrequency = stream->dev->clock_frequency;
>  		ctrl->bmFramingInfo = 0;
> -		ctrl->bPreferedVersion = 0;
> +		ctrl->bPreferredVersion = 0;
>  		ctrl->bMinVersion = 0;
>  		ctrl->bMaxVersion = 0;
>  	}
> @@ -325,7 +325,7 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
>  	if (size >= 34) {
>  		put_unaligned_le32(ctrl->dwClockFrequency, &data[26]);
>  		data[30] = ctrl->bmFramingInfo;
> -		data[31] = ctrl->bPreferedVersion;
> +		data[31] = ctrl->bPreferredVersion;
>  		data[32] = ctrl->bMinVersion;
>  		data[33] = ctrl->bMaxVersion;
>  	}

[snip]

> diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
> index d854cb19c42c..59167f0ed5c1 100644
> --- a/include/uapi/linux/usb/video.h
> +++ b/include/uapi/linux/usb/video.h
> @@ -448,7 +448,10 @@ struct uvc_streaming_control {
>  	__u32 dwMaxPayloadTransferSize;
>  	__u32 dwClockFrequency;
>  	__u8  bmFramingInfo;
> -	__u8  bPreferedVersion;
> +	union {
> +		__u8 bPreferredVersion;
> +		__u8 bPreferedVersion __attribute__((deprecated)); /* NOTYPO */
> +	} __attribute__((__packed__));

Quite interestingly this typo is part of the USB device class definition
for video devices (UVC) specification. I thus think we should keep using
the field name bPreferedVersion through the code, otherwise it wouldn't
match the spec.

>  	__u8  bMinVersion;
>  	__u8  bMaxVersion;
>  } __attribute__((__packed__));

[snip]

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Cleanup: replace prefered with preferred
       [not found] <20191022214208.211448-1-salyzyn@android.com>
  2019-10-23 10:22 ` [PATCH] Cleanup: replace prefered with preferred Laurent Pinchart
@ 2019-10-23 11:56 ` Jarkko Sakkinen
  2019-10-23 15:40   ` Mark Salyzyn
  1 sibling, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2019-10-23 11:56 UTC (permalink / raw)
  To: Mark Salyzyn
  Cc: linux-kernel, kernel-team, David S. Miller, Jonathan Corbet,
	Ard Biesheuvel, Harry Wentland, Leo Li, Alex Deucher,
	Christian König, David (ChunMing) Zhou, David Airlie,
	Daniel Vetter, VMware Graphics, Thomas Hellstrom,
	Laurent Pinchart, Mauro Carvalho Chehab, Trond Myklebust,
	Anna Schumaker, Alexander Aring, Jukka Rissanen,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, Ingo Molnar,
	Matthew Garrett, Hans de Goede, hersen wu, Roman Li,
	Maxim Martynov, David Ahern, Francesco Ruggeri,
	Linus Lüssing, Greg Kroah-Hartman, Feng Tang,
	Steven Rostedt (VMware),
	Andrew Morton, Rafael Aquini, netdev, linux-doc, linux-efi,
	amd-gfx, dri-devel, linux-media, linux-nfs, linux-bluetooth,
	linux-wpan

On Tue, Oct 22, 2019 at 02:41:45PM -0700, Mark Salyzyn wrote:
> Replace all occurrences of prefered with preferred to make future
> checkpatch.pl's happy.  A few places the incorrect spelling is
> matched with the correct spelling to preserve existing user space API.
> 
> Signed-off-by: Mark Salyzyn <salyzyn@android.com>

I'd fix such things when the code is otherwise change and scope this
patch only to Documentation/. There is no pragmatic benefit of doing
this for the code.

/Jarkko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Cleanup: replace prefered with preferred
  2019-10-23 11:56 ` Jarkko Sakkinen
@ 2019-10-23 15:40   ` Mark Salyzyn
  2019-10-24 12:26     ` Jani Nikula
                       ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Mark Salyzyn @ 2019-10-23 15:40 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-kernel, kernel-team, David S. Miller, Jonathan Corbet,
	Ard Biesheuvel, Harry Wentland, Leo Li, Alex Deucher,
	Christian König, David (ChunMing) Zhou, David Airlie,
	Daniel Vetter, VMware Graphics, Thomas Hellstrom,
	Laurent Pinchart, Mauro Carvalho Chehab, Trond Myklebust,
	Anna Schumaker, Alexander Aring, Jukka Rissanen,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, Ingo Molnar,
	Matthew Garrett, Hans de Goede, hersen wu, Roman Li,
	Maxim Martynov, David Ahern, Francesco Ruggeri,
	Linus Lüssing, Greg Kroah-Hartman, Feng Tang,
	Steven Rostedt (VMware),
	Andrew Morton, Rafael Aquini, netdev, linux-doc, linux-efi,
	amd-gfx, dri-devel, linux-media, linux-nfs, linux-bluetooth,
	linux-wpan

On 10/23/19 4:56 AM, Jarkko Sakkinen wrote:
> On Tue, Oct 22, 2019 at 02:41:45PM -0700, Mark Salyzyn wrote:
>> Replace all occurrences of prefered with preferred to make future
>> checkpatch.pl's happy.  A few places the incorrect spelling is
>> matched with the correct spelling to preserve existing user space API.
>>
>> Signed-off-by: Mark Salyzyn <salyzyn@android.com>
> I'd fix such things when the code is otherwise change and scope this
> patch only to Documentation/. There is no pragmatic benefit of doing
> this for the code.
>
> /Jarkko

The pragmatic benefit comes with the use of an ABI/API checker (which is 
a 'distro' thing, not a top of tree kernel thing) produces its map which 
is typically required to be co-located in the same tree as the kernel 
repository. Quite a few ABI/API update checkins result in a 
checkpatch.pl complaint about the misspelled elements being 
(re-)recorded due to proximity. We have a separate task to improve how 
it is tracked in Android to reduce milepost marker changes that result 
in sweeping changes to the database which would reduce the occurrences.

I will split this between pure and inert documentation/comments for now, 
with a followup later for the code portion which understandably is more 
controversial.

Cleanup is the least appreciated part of kernel maintenance ;-}.

Sincerely -- Mark Salyzyn


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Cleanup: replace prefered with preferred
  2019-10-23 15:40   ` Mark Salyzyn
@ 2019-10-24 12:26     ` Jani Nikula
  2019-10-24 17:30     ` Jarkko Sakkinen
  2019-10-24 18:47     ` Greg Kroah-Hartman
  2 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2019-10-24 12:26 UTC (permalink / raw)
  To: Mark Salyzyn, Jarkko Sakkinen
  Cc: linux-kernel, kernel-team, David S. Miller, Jonathan Corbet,
	Ard Biesheuvel, Harry Wentland, Leo Li, Alex Deucher,
	Christian König, David (ChunMing) Zhou, David Airlie,
	Daniel Vetter, VMware Graphics, Thomas Hellstrom,
	Laurent Pinchart, Mauro Carvalho Chehab, Trond Myklebust,
	Anna Schumaker, Alexander Aring, Jukka Rissanen,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, Ingo Molnar,
	Matthew Garrett, Hans de Goede, hersen wu, Roman Li,
	Maxim Martynov, David Ahern, Francesco Ruggeri,
	Linus Lüssing, Greg Kroah-Hartman, Feng Tang,
	Steven Rostedt (VMware),
	Andrew Morton, Rafael Aquini, netdev, linux-doc, linux-efi,
	amd-gfx, dri-devel, linux-media, linux-nfs, linux-bluetooth,
	linux-wpan

On Wed, 23 Oct 2019, Mark Salyzyn <salyzyn@android.com> wrote:
> I will split this between pure and inert documentation/comments for now, 
> with a followup later for the code portion which understandably is more 
> controversial.

Please split by driver/subsystem too, and it'll be all around much
easier for everyone.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Cleanup: replace prefered with preferred
  2019-10-23 15:40   ` Mark Salyzyn
  2019-10-24 12:26     ` Jani Nikula
@ 2019-10-24 17:30     ` Jarkko Sakkinen
  2019-10-24 18:47     ` Greg Kroah-Hartman
  2 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2019-10-24 17:30 UTC (permalink / raw)
  To: Mark Salyzyn
  Cc: linux-kernel, kernel-team, David S. Miller, Jonathan Corbet,
	Ard Biesheuvel, Harry Wentland, Leo Li, Alex Deucher,
	Christian König, David (ChunMing) Zhou, David Airlie,
	Daniel Vetter, VMware Graphics, Thomas Hellstrom,
	Laurent Pinchart, Mauro Carvalho Chehab, Trond Myklebust,
	Anna Schumaker, Alexander Aring, Jukka Rissanen,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, Ingo Molnar,
	Matthew Garrett, Hans de Goede, hersen wu, Roman Li,
	Maxim Martynov, David Ahern, Francesco Ruggeri,
	Linus Lüssing, Greg Kroah-Hartman, Feng Tang,
	Steven Rostedt (VMware),
	Andrew Morton, Rafael Aquini, netdev, linux-doc, linux-efi,
	amd-gfx, dri-devel, linux-media, linux-nfs, linux-bluetooth,
	linux-wpan

On Wed, Oct 23, 2019 at 08:40:59AM -0700, Mark Salyzyn wrote:
> On 10/23/19 4:56 AM, Jarkko Sakkinen wrote:
> > On Tue, Oct 22, 2019 at 02:41:45PM -0700, Mark Salyzyn wrote:
> > > Replace all occurrences of prefered with preferred to make future
> > > checkpatch.pl's happy.  A few places the incorrect spelling is
> > > matched with the correct spelling to preserve existing user space API.
> > > 
> > > Signed-off-by: Mark Salyzyn <salyzyn@android.com>
> > I'd fix such things when the code is otherwise change and scope this
> > patch only to Documentation/. There is no pragmatic benefit of doing
> > this for the code.
> > 
> > /Jarkko
> 
> The pragmatic benefit comes with the use of an ABI/API checker (which is a
> 'distro' thing, not a top of tree kernel thing) produces its map which is
> typically required to be co-located in the same tree as the kernel
> repository. Quite a few ABI/API update checkins result in a checkpatch.pl
> complaint about the misspelled elements being (re-)recorded due to
> proximity. We have a separate task to improve how it is tracked in Android
> to reduce milepost marker changes that result in sweeping changes to the
> database which would reduce the occurrences.
> 
> I will split this between pure and inert documentation/comments for now,
> with a followup later for the code portion which understandably is more
> controversial.
> 
> Cleanup is the least appreciated part of kernel maintenance ;-}.
> 
> Sincerely -- Mark Salyzyn

I'm a strong believer of "evolutionary" approach. Patch sets for the
most part (everything in the end has to be considered case by case, not
a strict rule) should have some functional changes involved.

What I do require for the parts that I maintain is that any new change
will result cleaner code base than the one that existed before that
change was applied. Again, there are some exceptions to this e.g.
circulating a firmware bug but this is my driving guideline as a
maintainer.

Doing cleanups just for cleanups can sometimes add unnecessary merge
conflicts when backporting patches to stable kernels. Thus, if you are
doing just a cleanup you should have extremely good reasons to do so.

/Jarkko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Cleanup: replace prefered with preferred
  2019-10-23 15:40   ` Mark Salyzyn
  2019-10-24 12:26     ` Jani Nikula
  2019-10-24 17:30     ` Jarkko Sakkinen
@ 2019-10-24 18:47     ` Greg Kroah-Hartman
  2 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2019-10-24 18:47 UTC (permalink / raw)
  To: Mark Salyzyn
  Cc: Jarkko Sakkinen, linux-kernel, kernel-team, David S. Miller,
	Jonathan Corbet, Ard Biesheuvel, Harry Wentland, Leo Li,
	Alex Deucher, Christian König, David (ChunMing) Zhou,
	David Airlie, Daniel Vetter, VMware Graphics, Thomas Hellstrom,
	Laurent Pinchart, Mauro Carvalho Chehab, Trond Myklebust,
	Anna Schumaker, Alexander Aring, Jukka Rissanen,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, Ingo Molnar,
	Matthew Garrett, Hans de Goede, hersen wu, Roman Li,
	Maxim Martynov, David Ahern, Francesco Ruggeri,
	Linus Lüssing, Feng Tang, Steven Rostedt (VMware),
	Andrew Morton, Rafael Aquini, netdev, linux-doc, linux-efi,
	amd-gfx, dri-devel, linux-media, linux-nfs, linux-bluetooth,
	linux-wpan

On Wed, Oct 23, 2019 at 08:40:59AM -0700, Mark Salyzyn wrote:
> On 10/23/19 4:56 AM, Jarkko Sakkinen wrote:
> > On Tue, Oct 22, 2019 at 02:41:45PM -0700, Mark Salyzyn wrote:
> > > Replace all occurrences of prefered with preferred to make future
> > > checkpatch.pl's happy.  A few places the incorrect spelling is
> > > matched with the correct spelling to preserve existing user space API.
> > > 
> > > Signed-off-by: Mark Salyzyn <salyzyn@android.com>
> > I'd fix such things when the code is otherwise change and scope this
> > patch only to Documentation/. There is no pragmatic benefit of doing
> > this for the code.
> > 
> > /Jarkko
> 
> The pragmatic benefit comes with the use of an ABI/API checker (which is a
> 'distro' thing, not a top of tree kernel thing) produces its map which is
> typically required to be co-located in the same tree as the kernel
> repository. Quite a few ABI/API update checkins result in a checkpatch.pl
> complaint about the misspelled elements being (re-)recorded due to
> proximity. We have a separate task to improve how it is tracked in Android
> to reduce milepost marker changes that result in sweeping changes to the
> database which would reduce the occurrences.

Requiring checkpatch spelling warnings to be correct based on function
names is crazy, you should fix your tools if you are requiring something
as looney as that :)

> I will split this between pure and inert documentation/comments for now,
> with a followup later for the code portion which understandably is more
> controversial.

Please break up per subsystem, like all trivial patches, as this
isn't anything special.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20191022214208.211448-1-salyzyn@android.com>
2019-10-23 10:22 ` [PATCH] Cleanup: replace prefered with preferred Laurent Pinchart
2019-10-23 11:56 ` Jarkko Sakkinen
2019-10-23 15:40   ` Mark Salyzyn
2019-10-24 12:26     ` Jani Nikula
2019-10-24 17:30     ` Jarkko Sakkinen
2019-10-24 18:47     ` Greg Kroah-Hartman

Linux-WPAN Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wpan/0 linux-wpan/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wpan linux-wpan/ https://lore.kernel.org/linux-wpan \
		linux-wpan@vger.kernel.org
	public-inbox-index linux-wpan

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wpan


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git