From: Anatolij Gustschin <agust@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 7/7] video_display: Add Xilinx LogiCore DP TX
Date: Sat, 26 May 2018 00:24:08 +0200 [thread overview]
Message-ID: <20180526002408.5f4b78db@crub> (raw)
In-Reply-To: <20180523121047.28330-7-mario.six@gdsys.cc>
Hi Mario,
Please test the patch with ./scripts/checkpatch.pl, there are warnings/
errors reported that need fixing:
...
total: 1 errors, 31 warnings, 26 checks, 2746 lines checked
On Wed, 23 May 2018 14:10:47 +0200
Mario Six mario.six at gdsys.cc wrote:
...
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index cf7ad281c3b..fa4ac715fcf 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_VIDEO_IVYBRIDGE_IGD) += ivybridge_igd.o
>
> obj-$(CONFIG_ATI_RADEON_FB) += ati_radeon_fb.o videomodes.o
> obj-$(CONFIG_ATMEL_HLCD) += atmel_hlcdfb.o
> +obj-$(CONFIG_LOGICORE_DP_TX) += logicore_dp_tx.o
Please add new driver entry sorted alphabetically. I know there are
already not sorted entries in the list here, but we shouldn't add more.
...
> diff --git a/drivers/video/logicore_dp_dpcd.h b/drivers/video/logicore_dp_dpcd.h
> new file mode 100644
> index 00000000000..68582945514
> --- /dev/null
> +++ b/drivers/video/logicore_dp_dpcd.h
...
> +struct main_stream_attributes {
> + /* Pixel clock of the stream (in Hz). */
> + u32 pixel_clock_hz;
> + /* Miscellaneous stream attributes 0 as specified by the DisplayPort
> + * 1.2 specification.
> + */
Please fix multi-line comment style throughout this patch, we use this style:
/*
* multi-line
* comment
*/
...
> +static u32 get_reg(struct udevice *dev, u32 reg)
> +{
> + struct dp_tx *dp_tx = dev_get_priv(dev);
> + u32 value = 0;
> + int res;
> +
> + // TODO error handling
please no C++ comments.
...
> +bool is_connected(struct udevice *dev)
> +{
shouldn't it be static?
...
> +
> +bool is_link_rate_valid(struct udevice *dev, u8 link_rate)
> +{
...
> +bool is_lane_count_valid(struct udevice *dev, u8 lane_count)
> +{
Are these functions supposed to be used externally? If not, please add
static. Otherwise move them to /* external API */ section.
...
> +int logicore_dp_tx_enable(struct udevice *dev, int panel_bpp,
> + const struct display_timing *timing)
> +{
should be static?
...
> +int logicore_dp_tx_probe(struct udevice *dev)
> +{
should be static?
...
> +struct dm_display_ops logicore_dp_tx_ops = {
> + .enable = logicore_dp_tx_enable,
> +};
should be static, too.
...
> diff --git a/drivers/video/logicore_dp_tx_regif.h b/drivers/video/logicore_dp_tx_regif.h
> new file mode 100644
> index 00000000000..c4105605c9b
> --- /dev/null
> +++ b/drivers/video/logicore_dp_tx_regif.h
...
> + /* core ID */
> + REG_VERSION = 0x0F8,
> + REG_CORE_ID = 0x0FC,
here a space and tabs follow '=', please use tabs only.
...
> + REG_USER_PIXEL_WIDTH = 0x1B8,
> + REG_USER_DATA_COUNT_PER_LANE =0x1BC,
spaces around '=', checkpatch will report style issues like this.
...
> +/*
> + * PHY_STATUS_ALL_LANES_READY_MASK seems zo be missing lanes 0 and 1 in
s/zo/to ?
Thanks,
--
Anatolij
next prev parent reply other threads:[~2018-05-25 22:24 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-23 12:10 [U-Boot] [PATCH v2 1/7] drivers: Add AXI uclass Mario Six
2018-05-23 12:10 ` [U-Boot] [PATCH v2 2/7] axi: Add ihs_axi driver Mario Six
2018-05-25 2:41 ` Simon Glass
2018-05-23 12:10 ` [U-Boot] [PATCH v2 3/7] axi: Add AXI sandbox driver and simple emulator Mario Six
2018-05-25 2:41 ` Simon Glass
2018-06-25 9:51 ` Mario Six
2018-05-23 12:10 ` [U-Boot] [PATCH v2 4/7] sandbox: Add and build AXI bus and device Mario Six
2018-05-23 12:10 ` [U-Boot] [PATCH v2 5/7] test: Add AXI test Mario Six
2018-05-25 2:41 ` Simon Glass
2018-06-25 9:51 ` Mario Six
2018-05-23 12:10 ` [U-Boot] [PATCH v2 6/7] cmd: Add axi command Mario Six
2018-05-25 2:41 ` Simon Glass
2018-06-25 10:01 ` Mario Six
2018-05-23 12:10 ` [U-Boot] [PATCH v2 7/7] video_display: Add Xilinx LogiCore DP TX Mario Six
2018-05-25 22:24 ` Anatolij Gustschin [this message]
2018-06-25 11:00 ` Mario Six
2018-05-25 2:41 ` [U-Boot] [PATCH v2 1/7] drivers: Add AXI uclass Simon Glass
2018-06-25 9:48 ` Mario Six
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=20180526002408.5f4b78db@crub \
--to=agust@denx.de \
--cc=u-boot@lists.denx.de \
/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