Re: [PATCH v5 2/2] clk: mediatek: Add drivers for MediaTek MT6735 main clock and reset drivers

[Yassine Oudjana <yassine.oudjana@gmail.com>]

On 10/10/2024 3:59 pm, AngeloGioacchino Del Regno wrote:
> Il 10/10/24 14:45, Yassine Oudjana ha scritto:
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>>
>> Add drivers for MT6735 apmixedsys, topckgen, infracfg and pericfg
>> clock and reset controllers. These provide the base clocks and resets
>> on the platform, enough to bring up all essential blocks including
>> PWRAP, MSDC and peripherals (UART, I2C, SPI).
>>
>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>> ---
>>   MAINTAINERS                                  |   4 +
>>   drivers/clk/mediatek/Kconfig                 |   9 +
>>   drivers/clk/mediatek/Makefile                |   1 +
>>   drivers/clk/mediatek/clk-mt6735-apmixedsys.c | 138 +++++++
>>   drivers/clk/mediatek/clk-mt6735-infracfg.c   | 107 +++++
>>   drivers/clk/mediatek/clk-mt6735-pericfg.c    | 124 ++++++
>>   drivers/clk/mediatek/clk-mt6735-topckgen.c   | 394 +++++++++++++++++++
>>   7 files changed, 777 insertions(+)
>>   create mode 100644 drivers/clk/mediatek/clk-mt6735-apmixedsys.c
>>   create mode 100644 drivers/clk/mediatek/clk-mt6735-infracfg.c
>>   create mode 100644 drivers/clk/mediatek/clk-mt6735-pericfg.c
>>   create mode 100644 drivers/clk/mediatek/clk-mt6735-topckgen.c
>>
> 
> You're so, so, so, so..... *so close* to get this one upstream.
> 
> Read the comments below and *please*, fix it and send a v6 *as soon as 
> you can*,
> so that we get a good chance to see this in v6.13 and so that you can 
> move on
> with upstreaming the other stuff that you got for this SoC.
> 
> ..snip...
> 
>> diff --git a/drivers/clk/mediatek/clk-mt6735-infracfg.c b/drivers/clk/ 
>> mediatek/clk-mt6735-infracfg.c
>> new file mode 100644
>> index 0000000000000..eceb13af3222a
>> --- /dev/null
>> +++ b/drivers/clk/mediatek/clk-mt6735-infracfg.c
>> @@ -0,0 +1,107 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2022 Yassine Oudjana <y.oudjana@protonmail.com>
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "clk-gate.h"
>> +#include "clk-mtk.h"
>> +
>> +#include <dt-bindings/clock/mediatek,mt6735-infracfg.h>
>> +#include <dt-bindings/reset/mediatek,mt6735-infracfg.h>
>> +
>> +#define INFRA_RST0            0x30
>> +#define INFRA_GLOBALCON_PDN0        0x40
>> +#define INFRA_PDN1            0x44
>> +#define    INFRA_PDN_STA            0x48
> 
> #define<tab>INFRA_PDN_STA .... I know this is a typo, but you'll have to 
> fix it.
> 
> Please replace the tabulation with a space, so that this is consistent 
> with all
> of the other definitions that you have in here.
> 
>> +
>> +#define RST_NR_PER_BANK            32
>> +
> 
> ..snip..
> 
>> diff --git a/drivers/clk/mediatek/clk-mt6735-pericfg.c b/drivers/clk/ 
>> mediatek/clk-mt6735-pericfg.c
>> new file mode 100644
>> index 0000000000000..6f298d5538782
>> --- /dev/null
>> +++ b/drivers/clk/mediatek/clk-mt6735-pericfg.c
>> @@ -0,0 +1,124 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2022 Yassine Oudjana <y.oudjana@protonmail.com>
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "clk-gate.h"
>> +#include "clk-mtk.h"
>> +
>> +#include <dt-bindings/clock/mediatek,mt6735-pericfg.h>
>> +#include <dt-bindings/reset/mediatek,mt6735-pericfg.h>
>> +
>> +#define PERI_GLOBALCON_RST0        0x00
>> +#define PERI_GLOBALCON_RST1        0x04
>> +#define PERI_GLOBALCON_PDN0_SET        0x08
>> +#define PERI_GLOBALCON_PDN0_CLR        0x10
>> +#define    PERI_GLOBALCON_PDN0_STA        0x18
> 
> same here
> 
>> +
>> +#define RST_NR_PER_BANK            32
> After which:
> 
> Reviewed-by: AngeloGioacchino Del Regno 
> <angelogioacchino.delregno@collabora.com>

Tried so hard to catch all typos but I guess some still had to make it 
through. One quick fix coming up!