diff mbox series

[1/5] sunxi: video: No double clock on DE2

Message ID 20200916141052.4808-2-m.cerveny@computer.org
State New
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series sunxi: video: Add V3S simple-framebuffer | expand

Commit Message

Martin Cerveny Sept. 16, 2020, 2:10 p.m. UTC
Weird code or comment. This is variant is tested on V3s.

Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
---
 drivers/video/sunxi/lcdc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Martin Cerveny Sept. 16, 2020, 4:25 p.m. UTC | #1
On Wed, 16 Sep 2020, Maxime Ripard wrote:

> On Wed, Sep 16, 2020 at 04:10:48PM +0200, Martin Cerveny wrote:
>> Weird code or comment. This is variant is tested on V3s.
>>
>> Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
>
> Generally speaking your commit logs are fairly concise, but it really
> becomes an issue when you're allegedly fixing a bug.
>
> There's a bunch of questions here that are completely up in the air:
>
>  - What issue are you actually trying to fix, how can one reproduce it
>  - You claim that there's no double clock on the DE2, according to
>    what?
>  - DE2 is used on way more SoCs than just the V3s, did you check/test
>    on those SoCs as well?
>
> In general, a good commit log should not explain what you're doing but
> *why* you're doing it. The what can be quite easily figured out from the
> patch content, the why can't today, and it will be even harder in a
> year's time.
>
> Maxime
>

I cannot issue more information on the problem:
- check original code - the "1x pll" was commented by #ifndef
   (only "2x pll" is computed)
- but original author added comment "/* No double clock on DE2 */"
- V3s does not support "2x pll" in TCON_CLK_REG

So I wrote where I tested this patch.
It should be discussed with original author.

Regards.

CC: Vasily Khoruzhick <anarsoul@gmail.com>
diff mbox series

Patch

diff --git a/drivers/video/sunxi/lcdc.c b/drivers/video/sunxi/lcdc.c
index 73033c3b85..b772947a6b 100644
--- a/drivers/video/sunxi/lcdc.c
+++ b/drivers/video/sunxi/lcdc.c
@@ -244,7 +244,6 @@  void lcdc_pll_set(struct sunxi_ccm_reg *ccm, int tcon, int dotclock,
 	 * not sync to higher frequencies.
 	 */
 	for (m = min_m; m <= max_m; m++) {
-#ifndef CONFIG_SUNXI_DE2
 		n = (m * dotclock) / step;
 
 		if ((n >= 9) && (n <= 127)) {
@@ -261,8 +260,8 @@  void lcdc_pll_set(struct sunxi_ccm_reg *ccm, int tcon, int dotclock,
 		/* These are just duplicates */
 		if (!(m & 1))
 			continue;
-#endif
 
+#ifndef CONFIG_SUNXI_DE2
 		/* No double clock on DE2 */
 		n = (m * dotclock) / (step * 2);
 		if ((n >= 9) && (n <= 127)) {
@@ -275,6 +274,7 @@  void lcdc_pll_set(struct sunxi_ccm_reg *ccm, int tcon, int dotclock,
 				best_double = 1;
 			}
 		}
+#endif
 	}
 
 #ifdef CONFIG_MACH_SUN6I
@@ -315,8 +315,7 @@  void lcdc_pll_set(struct sunxi_ccm_reg *ccm, int tcon, int dotclock,
 		writel(CCM_LCD_CH0_CTRL_GATE | CCM_LCD_CH0_CTRL_RST | pll,
 		       &ccm->lcd0_ch0_clk_cfg);
 #else
-		writel(CCM_LCD_CH0_CTRL_GATE | CCM_LCD_CH0_CTRL_RST | pll,
-		       &ccm->lcd0_clk_cfg);
+		writel(CCM_LCD0_CTRL_GATE | pll, &ccm->lcd0_clk_cfg);
 #endif
 	}
 #ifndef CONFIG_SUNXI_DE2