Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Andrew Jeffery <andrew@codeconstruct.com.au>]

On Tue, 2024-08-20 at 01:52 +0000, Ryan Chen wrote:
> > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > AST2700
> > 
> > On Mon, 2024-08-19 at 03:05 +0000, Ryan Chen wrote:
> > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support
> > > > > for
> > > > > AST2700
> > > > > 
> > > > > On Wed, 2024-08-14 at 06:35 +0000, Ryan Chen wrote:
> > > > > > > 
> > > > > > > Didn't I see in another patch one die is cpu and one is
> > > > > > > io?
> > > > > > > Use
> > > > > > > those in the compatible rather than 0 and 1 if so.
> > > > > > > 
> > > > > > Sorry, I want to align with our datasheet description.
> > > > > > It will but scu0 and scu1 register setting.
> > > > > 
> > > > > Can we document that relationship in the binding? Rob's
> > > > > suggestion
> > > > > seems more descriptive.
> > > > Hello,
> > > > 	Do you want me document it in yaml file or just in
> > > > commit
> > > > message?
> > > 
> > > Hello Rob, Andrew,
> > > 	I will add in yaml file in description. Like following.
> > > 
> > > description:
> > >   The Aspeed System Control Unit manages the global behaviour of
> > > the
> > > SoC,
> > >   configuring elements such as clocks, pinmux, and reset.
> > > +  In AST2700, it has two soc combination. Each soc include its
> > > own
> > > scu register control.
> > > +  ast2700-scu0 for soc0, ast2700-scu1 for soc1.
> > > 
> > > Is that will be better way ?
> > 
> > What Rob is suggesting is to add the compatibles "aspeed,ast2700-
> > scu-
> > cpu" and "aspeed,ast2700-scu-io", and then in the description say
> > something like:
> > 
> >    The AST2700 integrates both a CPU and an IO die, each with their
> > own
> >    SCU. The "aspeed,ast2700-scu-cpu" and "aspeed,ast2700-scu-io"
> >    compatibles correspond to SCU0 and SCU1 respectively.
> > 
> Hello Andrew,
> 	Sorry, for correspond for ast2700 datasheet, the description
> is scu0/scu1.
> 	System Control Unit #0 (SCU0)/ System Control Unit #1 (SCU1)
> why not we
> 	Keep align with datasheet statement?

Well, IMO we have an opportunity do better in the compatibles. I expect
we should take advantage of it. As some support for Rob's suggestion,
the datasheet chapter for SCU1 calls it "SCUIO" in the first sentence
of the description. Further, there are only two SCUs, and I don't think
the mapping of "cpu" to 0 and "io" to 1 is too difficult to keep track
of, certainly not if it's written in the binding documentation (as long
as these names are accurate!). The argument works both ways but I don't
think it's contentious that using the indexes is _less_ descriptive.

That said, this is just my semi-informed opinion. It's up to you to
decide what names you're going to push for. Rob's suggestion seems
reasonable to me though.

Andrew