Re: [PATCH 2/2] clk: en7523: fix scuclk io region for upcoming pinctrl

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 09/07/24 09:48, Lorenzo Bianconi ha scritto:
> EN7581 clock driver shares the IO region with the upcoming pinctrl
> driver for Airoha EN7581 SoC. Fix it by reducing the clk mapped
> region to only used registers in order to not overlap with pinctrl
> one. This change is not introducing any backward compatibility issue
> since the EN7581 dts is not upstream yet.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>   drivers/clk/clk-en7523.c | 23 +++++++++++++++--------
>   1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
> index b20e56337a6b..d9ecbb6bf55a 100644
> --- a/drivers/clk/clk-en7523.c
> +++ b/drivers/clk/clk-en7523.c
> @@ -31,7 +31,14 @@
>   #define   REG_RESET_CONTROL_PCIE1	BIT(27)
>   #define   REG_RESET_CONTROL_PCIE2	BIT(26)
>   /* EN7581 */
> -#define REG_CRYPTO_CLKSRC2		0x20c
> +#define REG_GSW_CLK_DIV_SEL2		0x00
> +#define REG_EMI_CLK_DIV_SEL2		0x04
> +#define REG_BUS_CLK_DIV_SEL2		0x08
> +#define REG_SPI_CLK_DIV_SEL2		0x10
> +#define REG_SPI_CLK_FREQ_SEL2		0x14
> +#define REG_NPU_CLK_DIV_SEL2		0x48
> +#define REG_CRYPTO_CLKSRC2		0x58
> +
>   #define REG_PCIE0_MEM			0x00
>   #define REG_PCIE0_MEM_MASK		0x04
>   #define REG_PCIE1_MEM			0x08
> @@ -203,7 +210,7 @@ static const struct en_clk_desc en7581_base_clks[] = {
>   		.id = EN7523_CLK_GSW,
>   		.name = "gsw",
>   
> -		.base_reg = REG_GSW_CLK_DIV_SEL,
> +		.base_reg = REG_GSW_CLK_DIV_SEL2,

This is practically just commit noise :-)

You are adding the en7581_base_clks[] in patch [1/2] with the wrong base register,
then fixing it here ... and that's wrong.

Please squash the two patches.

Cheers,
Angelo

>   		.base_bits = 1,
>   		.base_shift = 8,
>   		.base_values = gsw_base,
> @@ -217,7 +224,7 @@ static const struct en_clk_desc en7581_base_clks[] = {
>   		.id = EN7523_CLK_EMI,
>   		.name = "emi",
>   
> -		.base_reg = REG_EMI_CLK_DIV_SEL,
> +		.base_reg = REG_EMI_CLK_DIV_SEL2,
>   		.base_bits = 2,
>   		.base_shift = 8,
>   		.base_values = emi7581_base,
> @@ -231,7 +238,7 @@ static const struct en_clk_desc en7581_base_clks[] = {
>   		.id = EN7523_CLK_BUS,
>   		.name = "bus",
>   
> -		.base_reg = REG_BUS_CLK_DIV_SEL,
> +		.base_reg = REG_BUS_CLK_DIV_SEL2,
>   		.base_bits = 1,
>   		.base_shift = 8,
>   		.base_values = bus_base,
> @@ -245,13 +252,13 @@ static const struct en_clk_desc en7581_base_clks[] = {
>   		.id = EN7523_CLK_SLIC,
>   		.name = "slic",
>   
> -		.base_reg = REG_SPI_CLK_FREQ_SEL,
> +		.base_reg = REG_SPI_CLK_FREQ_SEL2,
>   		.base_bits = 1,
>   		.base_shift = 0,
>   		.base_values = slic_base,
>   		.n_base_values = ARRAY_SIZE(slic_base),
>   
> -		.div_reg = REG_SPI_CLK_DIV_SEL,
> +		.div_reg = REG_SPI_CLK_DIV_SEL2,
>   		.div_bits = 5,
>   		.div_shift = 24,
>   		.div_val0 = 20,
> @@ -260,7 +267,7 @@ static const struct en_clk_desc en7581_base_clks[] = {
>   		.id = EN7523_CLK_SPI,
>   		.name = "spi",
>   
> -		.base_reg = REG_SPI_CLK_DIV_SEL,
> +		.base_reg = REG_SPI_CLK_DIV_SEL2,
>   
>   		.base_value = 400000000,
>   
> @@ -272,7 +279,7 @@ static const struct en_clk_desc en7581_base_clks[] = {
>   		.id = EN7523_CLK_NPU,
>   		.name = "npu",
>   
> -		.base_reg = REG_NPU_CLK_DIV_SEL,
> +		.base_reg = REG_NPU_CLK_DIV_SEL2,
>   		.base_bits = 2,
>   		.base_shift = 8,
>   		.base_values = npu7581_base,