On 3/8/22 13:50, Ahamed Husni wrote: > Hello Hans, > > On Mon, Mar 7, 2022 at 12:58 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote: >> >> Hi Husni, >> >> Thank you for the patch. >> >> The Subject line needs some work: either name the source ('av7110_av.c:') or >> driver ('av7110:'), but not both. Also just stick to lower case, so: >> "media: av7110_av.c: fix switch indentation" >> >> That gives all the relevant information, and is a lot shorter. > Noted with thanks. I'll update the subject line in the V2 of the patch. > >> >> On 2/25/22 16:56, Husni Faiz wrote: >>> This patch fixes "switch and case should be at the same indent" >>> checkpatch error. >>> >>> Signed-off-by: Husni Faiz <ahamedhusni73@xxxxxxxxx> >>> --- >>> drivers/staging/media/av7110/av7110_av.c | 30 ++++++++++++------------ >>> 1 file changed, 15 insertions(+), 15 deletions(-) >>> >>> diff --git a/drivers/staging/media/av7110/av7110_av.c b/drivers/staging/media/av7110/av7110_av.c >>> index 91f4866c7e59..1d42862e9669 100644 >>> --- a/drivers/staging/media/av7110/av7110_av.c >>> +++ b/drivers/staging/media/av7110/av7110_av.c >>> @@ -770,22 +770,22 @@ static void p_to_t(u8 const *buf, long int length, u16 pid, u8 *counter, >>> if (length > 3 && >>> buf[0] == 0x00 && buf[1] == 0x00 && buf[2] == 0x01) >>> switch (buf[3]) { >>> - case PROG_STREAM_MAP: >>> - case PRIVATE_STREAM2: >>> - case PROG_STREAM_DIR: >>> - case ECM_STREAM : >>> - case EMM_STREAM : >>> - case PADDING_STREAM : >>> - case DSM_CC_STREAM : >>> - case ISO13522_STREAM: >>> - case PRIVATE_STREAM1: >>> - case AUDIO_STREAM_S ... AUDIO_STREAM_E: >>> - case VIDEO_STREAM_S ... VIDEO_STREAM_E: >>> - pes_start = 1; >>> - break; >>> + case PROG_STREAM_MAP: >>> + case PRIVATE_STREAM2: >>> + case PROG_STREAM_DIR: >>> + case ECM_STREAM : >>> + case EMM_STREAM : >>> + case PADDING_STREAM : >>> + case DSM_CC_STREAM : >>> + case ISO13522_STREAM: >>> + case PRIVATE_STREAM1: >>> + case AUDIO_STREAM_S ... AUDIO_STREAM_E: >>> + case VIDEO_STREAM_S ... VIDEO_STREAM_E: >>> + pes_start = 1; >>> + break; >>> >>> - default: >>> - break; >>> + default: >>> + break; >>> } >>> >>> while (c < length) { >> >> Running checkpatch.pl over this patch give me: >> >> ERROR: space prohibited before that ':' (ctx:WxE) >> #40: FILE: drivers/staging/media/av7110/av7110_av.c:776: >> + case ECM_STREAM : >> ^ >> >> ERROR: space prohibited before that ':' (ctx:WxE) >> #41: FILE: drivers/staging/media/av7110/av7110_av.c:777: >> + case EMM_STREAM : >> ^ >> >> ERROR: space prohibited before that ':' (ctx:WxE) >> #42: FILE: drivers/staging/media/av7110/av7110_av.c:778: >> + case PADDING_STREAM : >> ^ >> >> ERROR: space prohibited before that ':' (ctx:WxE) >> #43: FILE: drivers/staging/media/av7110/av7110_av.c:779: >> + case DSM_CC_STREAM : >> ^ >> Can you fix that as well in a v2 of this patch? > It seems that these spaces are deliberately added by the author to > keep the case statements' colons aligned. > Some other lines in the file where the same approach has been taken > are [line 598,599,600,601] and [line 662, 663, 664, 665]. > Should we leave these spaces as it is? Either just fix it here, or post a second patch that removes the spaces throughout this driver. It's a very old driver, predating the much more strict enforcement of coding style that we have today. Regards, Hans > > Thanks, > Husni.