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
prev parent 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