Re: [PATCH 4/5] clk: en7523: Add reset-controller support for EN7581 SoC

From: Lorenzo Bianconi <lorenzo@kernel.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: linux-clk@vger.kernel.org, p.zabel@pengutronix.de,
	mturquette@baylibre.com, sboyd@kernel.org,
	lorenzo.bianconi83@gmail.com, conor@kernel.org,
	linux-arm-kernel@lists.infradead.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, nbd@nbd.name, john@phrozen.org,
	dd@embedd.com, catalin.marinas@arm.com, will@kernel.org,
	upstream@airoha.com
Subject: Re: [PATCH 4/5] clk: en7523: Add reset-controller support for EN7581 SoC
Date: Thu, 16 May 2024 13:22:16 +0200	[thread overview]
Message-ID: <ZkXsaLylQOTis_wF@lore-desk> (raw)
In-Reply-To: <97b1fbfc-e059-46c7-9bb1-75cdaa2d2159@collabora.com>

[-- Attachment #1.1: Type: text/plain, Size: 2234 bytes --]

> Il 15/05/24 14:58, Lorenzo Bianconi ha scritto:
> > Introduce reset API support to EN7581 clock driver.
> > 
> > Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> Heh, that's exactly the usual MediaTek reset controller :-D
> 
> > ---
> >   drivers/clk/clk-en7523.c | 96 +++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 94 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
> > index 381605be333f..18798b692b68 100644
> > --- a/drivers/clk/clk-en7523.c
> > +++ b/drivers/clk/clk-en7523.c
> > @@ -6,6 +6,7 @@
> >   #include <linux/of.h>
> >   #include <linux/of_device.h>
> >   #include <linux/platform_device.h>
> > +#include <linux/reset-controller.h>
> >   #include <dt-bindings/clock/en7523-clk.h>
> >   #define REG_PCI_CONTROL			0x88
> > @@ -65,8 +66,18 @@ struct en_clk_gate {
> >   	struct clk_hw hw;
> >   };
> > +#define RST_NR_PER_BANK		32
> 
> Please move this definition at the beginning of this file, grouping that with
> the others.

ack, I will fix it.

> 
> ..snip...
> 
> > @@ -456,12 +542,14 @@ static int en7523_clk_probe(struct platform_device *pdev)
> >   	en7523_register_clocks(&pdev->dev, clk_data, base, np_base);
> >   	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> > -	if (r)
> > +	if (r) {
> >   		dev_err(&pdev->dev,
> >   			"could not register clock provider: %s: %d\n",
> >   			pdev->name, r);
> > +		return r;
> > +	}
> > -	return r;
> > +	return en7523_reset_register(&pdev->dev, np_base, soc_data);
> 
> If en7523_reset_register fails, you want to call of_clk_del_provider(), so
> you can't just return like this...

ack, I will fix it.

Regards,
Lorenzo

> 
> Cheers,
> Angelo
> 
> >   }
> >   static const struct en_clk_soc_data en7523_data = {
> > @@ -480,6 +568,10 @@ static const struct en_clk_soc_data en7581_data = {
> >   		.unprepare = en7581_pci_unprepare,
> >   		.disable = en7581_pci_disable,
> >   	},
> > +	.reset_data = {
> > +		.base_addr = REG_RESET_CONTROL2,
> > +		.n_banks = 2,
> > +	},
> >   	.hw_init = en7581_clk_hw_init,
> >   };
> 
> 
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel