mbox series

[v2,0/2] reset: Add ast2600 support

Message ID 20191129000827.650566-1-joel@jms.id.au
Headers show
Series reset: Add ast2600 support | expand

Message

Joel Stanley Nov. 29, 2019, 12:08 a.m. UTC 
This series adds ast2600 support to the reset-simple driver, and updates
the device tree documentation for the LPC bindings.

v2 fixes a typo in the bindings that Andrew found.

Brad Bishop (1):
  reset: simple: Add AST2600 compatibility string

Joel Stanley (1):
  dt-bindings: mfd: Add ast2600 to ASPEED LPC

 Documentation/devicetree/bindings/mfd/aspeed-lpc.txt | 11 +++++++++--
 drivers/reset/reset-simple.c                         |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Philipp Zabel Dec. 2, 2019, 12:53 p.m. UTC | #1 
On Fri, 2019-11-29 at 10:38 +1030, Joel Stanley wrote:
> From: Brad Bishop <bradleyb@fuzziesquirrel.com>
> 
> The AST2600 SoC contains the same LPC register set as the AST2500.

If the LPC register set is exactly the same, shouldn't AST2600 reuse the
AST2500 compatible, i.e.:
	compatible = "aspeed,ast2600-lpc-reset", "aspeed,ast2500-lpc-reset";
?

> 
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  drivers/reset/reset-simple.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index 067e7e7b34f1..795c9063fe7b 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -125,6 +125,7 @@ static const struct of_device_id reset_simple_dt_ids[] = {
>  		.data = &reset_simple_active_low },
>  	{ .compatible = "aspeed,ast2400-lpc-reset" },
>  	{ .compatible = "aspeed,ast2500-lpc-reset" },
> +	{ .compatible = "aspeed,ast2600-lpc-reset" },
>  	{ .compatible = "bitmain,bm1880-reset",
>  		.data = &reset_simple_active_low },
>  	{ .compatible = "snps,dw-high-reset" },

regards
Philipp
Brad Bishop Dec. 12, 2019, 2:51 p.m. UTC | #2 
Hi Philipp.  Thanks for your time.

> On Dec 2, 2019, at 7:53 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> 
> On Fri, 2019-11-29 at 10:38 +1030, Joel Stanley wrote:
>> From: Brad Bishop <bradleyb@fuzziesquirrel.com>
>> 
>> The AST2600 SoC contains the same LPC register set as the AST2500.
> 
> If the LPC register set is exactly the same, shouldn't AST2600 reuse the
> AST2500 compatible, i.e.:
> 	compatible = "aspeed,ast2600-lpc-reset", "aspeed,ast2500-lpc-reset";
> ?

I’m not sure.  I let what was already there be my guide - the ast2500 LPC registers are the same as the ast2400 as well and those got their own compatibles.  Is there a guideline written down somewhere that backs your thinking up?

thanks - brad
Philipp Zabel Dec. 12, 2019, 3:17 p.m. UTC | #3 
Hi Brad,

On Thu, 2019-12-12 at 09:51 -0500, Brad Bishop wrote:
> Hi Philipp.  Thanks for your time.
> 
> > On Dec 2, 2019, at 7:53 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > 
> > On Fri, 2019-11-29 at 10:38 +1030, Joel Stanley wrote:
> > > From: Brad Bishop <bradleyb@fuzziesquirrel.com>
> > > 
> > > The AST2600 SoC contains the same LPC register set as the AST2500.
> > 
> > If the LPC register set is exactly the same, shouldn't AST2600 reuse the
> > AST2500 compatible, i.e.:
> > 	compatible = "aspeed,ast2600-lpc-reset", "aspeed,ast2500-lpc-reset";
> > ?
> 
> I’m not sure.  I let what was already there be my guide - the ast2500
> LPC registers are the same as the ast2400 as well and those got their
> own compatibles.  Is there a guideline written down somewhere that
> backs your thinking up?

I read section 2.3.1 "compatible" of the DeviceTree Specification [1] as
supporting that view. If all three LPC reset controllers are in fact
identical, I would argue that both ast2500 and ast2600 are compatible to
ast2400 and should be specified as:
	compatible = "aspeed,ast2500-lpc-reset", "aspeed,ast2400-lpc-reset";
and:
	compatible = "aspeed,ast2600-lpc-reset", "aspeed,ast2400-lpc-reset";
respectively.

[1] https://github.com/devicetree-org/devicetree-specification/releases/download/v0.2/devicetree-specification-v0.2.pdf

regards
Philipp