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@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