lkml.org 
[lkml]   [2021]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2] drivers/cdrom: improved ioctl for media change detection
Date
Hey Phil,

thanks for taking the time to look at this and give feedback.

> Dear Lukas,
>
> Thank you for the patch, much appreciated and looks great. One very
> minor thing though has jumped out at me after running checkpatch though:
>
> > --- a/include/uapi/linux/cdrom.h
> > +++ b/include/uapi/linux/cdrom.h
> > @@ -147,6 +147,8 @@
> > #define CDROM_NEXT_WRITABLE 0x5394 /* get next writable block */
> > #define CDROM_LAST_WRITTEN 0x5395 /* get last block written on disc */
> >
> > +#define CDROM_TIMED_MEDIA_CHANGE 0x5396 /* get the timestamp of the last media change */
> > +
> > /*******************************************************
> > * CDROM IOCTL structures
> > *******************************************************/
> > @@ -295,6 +297,19 @@ struct cdrom_generic_command
> > };
> > };
> >
> > +/* This struct is used by CDROM_TIMED_MEDIA_CHANGE */
> > +struct cdrom_timed_media_change_info
> > +{
>
> This opening brace should be on the same line as the struct definition
> as per current style rules.

I also noted that checkpatch reported this and that it is technically a style breach,
however I kept the brace in the newline to be consistent with all the other
cdrom ioctl structs defined above this in the same file for consistency.
I have no strong feelings about this, though, so we could change this.

> I also got a checkpatch warning about ENOSYS being used as an error
> value, but I can see this usage is standard in the driver and not a
> problem so no issue with that.
>
> I will review and test properly after work tomorrow (being new to the
> role I'd like to make sure I'm taking the proper time), but I have no
> doubt it will work fine. Assuming it does I will be happy to accept the
> patch with the above brace change. Thanks again.
> Regards,
> Phil

Best regards,
Lukas

\
 
 \ /
  Last update: 2021-09-09 20:05    [W:0.310 / U:0.228 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site