linux-kernel - Re: [PATCH 01/14] clk: rockchip: add more clk ids for rv1108

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3065556.yAAG1ix4ad@phil>
Date:   Tue, 01 Aug 2017 01:27:45 +0200
From:   Heiko Stuebner <heiko@...ech.de>
To:     linux-rockchip@...ts.infradead.org
Cc:     Andy Yan <andy.yan@...k-chips.com>, devicetree@...r.kernel.org,
        shawn.lin@...k-chips.com, zhangqing@...k-chips.com,
        linux-kernel@...r.kernel.org, robh+dt@...nel.org,
        linux-clk@...r.kernel.org
Subject: Re: [PATCH 01/14] clk: rockchip: add more clk ids for rv1108

Hi Andy,

Am Montag, 31. Juli 2017, 18:06:42 CEST schrieb Andy Yan:
> From: Elaine Zhang <zhangqing@...k-chips.com>
> 
> Added the missing clock ids, make the clock more complete.
> 
> Signed-off-by: Elaine Zhang <zhangqing@...k-chips.com>
> Signed-off-by: Andy Yan <andy.yan@...k-chips.com>

this clock-id patch is way to overloaded. Please split into 3 parts:

- completely new clock-ids
- changes to existing clock-ids
  This also needs a very good reasoning, as such constant ids are
  considered part of api and should not change to prevent breakage
  with old devicetrees. So please double-check if the rename is really
  necessary.
- cosmetics ... aka the indentation stuff

[...]

>  /* aclk gates */
>  #define ACLK_DMAC			192
> -#define ACLK_PRE			193
> +#define ACLK_BUS			193

This is one such case where you need a very good reason

[...]

> @@ -59,18 +120,30 @@
>  #define PCLK_I2C2			261
>  #define PCLK_I2C3			262
>  #define PCLK_SPI			263
> -#define PCLK_SFC			264

why is this clock going away?

>  #define PCLK_UART0			265
>  #define PCLK_UART1			266
>  #define PCLK_UART2			267
>  #define PCLK_TSADC			268
> -#define PCLK_PWM			269
> +#define PCLK_PWM1			269

In your pwm patch, all blocks use the same clocks, so
this rename should be unnecessary


>  /* hclk gates */
>  #define HCLK_I2S0_8CH			320
> -#define HCLK_I2S1_8CH			321
> +#define HCLK_I2S1_2CH			321

Again reasoning required (that i2s1 has 2 channels
but not 8 should be ok for that)

>  #define HCLK_I2S2_2CH			322
>  #define HCLK_NANDC			323
>  #define HCLK_SDMMC			324

Heiko