Reply: Re: [PATCH v3 1/3] drm/loongson: Add DRM Driver for Loongson 7A1000 bridge chip

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sam,

Thank you very much for your attention to our driver.
I will consider your opinion, The next version is due in the near future.

> -----Original Messages-----
&gt; From: "Sam Ravnborg" <sam@xxxxxxxxxxxx>
&gt; Sent Time: 2021-07-28 01:41:30 (Wednesday)
&gt; To: "Daniel Vetter" <daniel@xxxxxxxx>
&gt; Subject: Re: [PATCH v3 1/3] drm/loongson: Add DRM Driver for Loongson 7A1000 bridge chip
&gt; 
&gt; Hi Chenyang,
&gt; 
&gt; I browsed the code on lore and noticed a few things and thought it
&gt; better to bring it to your attention now.
&gt; 
&gt; The general structure of the drivers seems good and coding style is
&gt; fine. The feedback is mostly stuff we have decided to do different over
&gt; time, so likely because you based the driver on some older driver.
&gt; And it can be hard to follow all the refactoring going on - something
&gt; that you get for free (almost) when is is mainlined.
&gt; 
&gt; 1) Use drm_ for logging whereever possible (need drm_device)
&gt; 2) Do not use irq mid-layer support in drm_driver, it is about to be
&gt;    legacy only. In other words avoid using drm_irq* stuff.
&gt; 3) Look at drm_drv to see code snippet how to use the drmm*
&gt;    infrastructure. It will save you some cleanup and in general make the
&gt;    driver more stable
&gt; 4) Sort includes alphabetically, and split them on in blocks.
&gt;    <linux *=""> is one block
&gt;    <newline>
&gt;    <drm drm_*=""> is next block
&gt; 5) Put entry in makefile in alphabetical order
&gt; 6) You most like can use the simple_encoder stuff we have today
&gt; 7) The *_load and *_unlod names where used in the past. Maybe be
&gt;    inspired by some newer driver here. _load functiosn is something used
&gt;    by legacy drivers so it confuses me a little.
&gt; 
&gt; I look forward to see next revision of the patch-set.
&gt; And sorry for not providing these high-level feedback issues before - I
&gt; have not had time to look at your driver.
&gt; 
&gt; 	Sam


------------------------------
LiChenyang</drm></newline></linux></daniel@xxxxxxxx></sam@xxxxxxxxxxxx>
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux