public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Chethan C <mail.chethanc@gmail.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Hans Verkuil <hverkuil+cisco@kernel.org>,
	Kees Cook <kees@kernel.org>, Petr Mladek <pmladek@suse.com>,
	Osama Albahrani <osalbahr@gmail.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	linux-media@vger.kernel.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] staging: media: av7110: fix coding style
Date: Mon, 23 Mar 2026 10:58:58 +0200	[thread overview]
Message-ID: <acEA0iSrMBudd1sC@ashevche-desk.local> (raw)
In-Reply-To: <20260321074614.541740-1-mail.chethanc@gmail.com>

On Sat, Mar 21, 2026 at 01:16:01PM +0530, Chethan C wrote:
> Fixed Indentation, Alignment issues reported by checkpatch.pl.
> 
> Renamed enum av7110_rec_play_state, av7110_type_rec_play_format,
> av7110_encoder_command from camel case to upper case underscore
> style to comply kernel style guidelines.

Some changes are questionable, some need improvement, see a bit below.

...

> +	ret = av7110_fw_cmd(av7110, COMTYPE_ENCODER, AV7110_SET_MONITOR_TYPE,
>  			    1, (u16)av7110->display_ar);

Also check if you need a casting.

>  	if (ret < 0)
>  		pr_err("unable to set aspect ratio\n");

...

> -	ret = av7110_fw_cmd(av7110, COMTYPE_ENCODER, SetWSSConfig, 2, 2, wss_cfg_4_3);
> +	ret = av7110_fw_cmd(av7110, COMTYPE_ENCODER, AV7110_SET_WSS_CONFIG, 2, 2, wss_cfg_4_3);

The media subsystem is strict about 80 character limit. Yes, the original is
also long, but this makes it worse. Same for other similar cases.

>  	if (ret < 0)
>  		pr_err("unable to configure 4:3 wss\n");

...

> -		ret = av7110_fw_cmd(av7110, COMTYPE_REC_PLAY, __Record, 2, AudioPES, 0);
> +		ret = av7110_fw_cmd(av7110,
> +				    COMTYPE_REC_PLAY,
> +				    AV7110_REC_PLAY_RECORD,
> +				    2,
> +				    AV7110_AUDIO_PES, 0);

Here and in some other places out of a sudden the style is changed to "one
parameter per line", why?

...

>  enum av7110_type_rec_play_format {
> -	RP_None,
> -	AudioPES,
> -	AudioMp2,
> -	AudioPCM,
> -	VideoPES,
> -	AV_PES
> +	AV7110_RP_NONE,
> +	AV7110_AUDIO_PES,
> +	AV7110_AUDIO_MP2,
> +	AV7110_AUDIO_PCM,
> +	AV7110_VIDEO_PES,
> +	AV7110_AV_PES

While there is no comma in the original, this is not a terminator entry, so
while at it, add trailing comma.

>  };

...

>  enum av7110_pid_command {
> -	MultiPID,
> -	VideoPID,
> -	AudioPID,
> -	InitFilt,
> -	FiltError,
> -	NewVersion,
> -	CacheError,
> -	AddPIDFilter,
> -	DelPIDFilter,
> -	Scan,
> -	SetDescr,
> -	SetIR,
> -	FlushTSQueue
> +	AV7110_MULTI_PID,
> +	AV7110_VIDEO_PID,
> +	AV7110_AUDIO_PID,
> +	AV7110_INIT_FILT,
> +	AV7110_FILT_ERROR,
> +	AV7110_NEW_VERSION,
> +	AV7110_CACHE_ERROR,
> +	AV7110_ADD_PID_FILTER,
> +	AV7110_DEL_PID_FILTER,
> +	AV7110_SCAN,
> +	AV7110_SET_DESCR,
> +	AV7110_SET_IR,
> +	AV7110_FLUSH_TS_QUEUE
>  };

Ditto.

...

>  enum av7110_encoder_command {
> -	SetVidMode,
> -	SetTestMode,
> -	LoadVidCode,
> -	SetMonitorType,
> -	SetPanScanType,
> -	SetFreezeMode,
> -	SetWSSConfig
> +	AV7110_SET_VID_MODE,
> +	AV7110_SET_TEST_MODE,
> +	AV7110_LOAD_VID_CODE,
> +	AV7110_SET_MONITOR_TYPE,
> +	AV7110_SET_PANSCAN_TYPE,
> +	AV7110_SET_FREEZE_MODE,
> +	AV7110_SET_WSS_CONFIG
>  };


Ditto.

>  enum av7110_rec_play_state {
> -	__Record,
> -	__Stop,
> -	__Play,
> -	__Pause,
> -	__Slow,
> -	__FF_IP,
> -	__Scan_I,
> -	__Continue
> +	AV7110_REC_PLAY_RECORD,
> +	AV7110_REC_PLAY_STOP,
> +	AV7110_REC_PLAY_PLAY,
> +	AV7110_REC_PLAY_PAUSE,
> +	AV7110_REC_PLAY_SLOW,
> +	AV7110_REC_PLAY_FF_IP,
> +	AV7110_REC_PLAY_SCAN_I,
> +	AV7110_REC_PLAY_CONTINUE
>  };

Ditto.

-- 
With Best Regards,
Andy Shevchenko



      reply	other threads:[~2026-03-23  8:59 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-21  7:46 [PATCH v3] staging: media: av7110: fix coding style Chethan C
2026-03-23  8:58 ` Andy Shevchenko [this message]

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=acEA0iSrMBudd1sC@ashevche-desk.local \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hverkuil+cisco@kernel.org \
    --cc=kees@kernel.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=mail.chethanc@gmail.com \
    --cc=mchehab@kernel.org \
    --cc=osalbahr@gmail.com \
    --cc=pmladek@suse.com \
    /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