| Message ID | 20200916141052.4808-2-m.cerveny@computer.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | sunxi: video: Add V3S simple-framebuffer | expand |
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 --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
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(-)