Re: [PATCH 1/6] dt-bindings: mfd: add binding for Apple Mac System Management Controller

[Hector Martin <marcan@xxxxxxxxx>]

On 06/09/2022 22.43, Russell King (Oracle) wrote:
> On Tue, Sep 06, 2022 at 10:28:25PM +0900, Hector Martin wrote:
>> Ultimately, we're working with a reverse engineered platform here, and
>> DTs will inevitaby be incremental. But in this particular case, of
>> hardware that has no known useful purpose to an OS, I don't see the
>> point in gratuitously describing it. And besides, the way we set things
>> up, forward-compatible DT upgrades are trivial, and no actual user on
>> this platform is going to be stuck with an old DT and newer software (if
>> their software supports the platform properly, and that takes more than
>> the relatively trivial DT upgrade stuff anyway). I'm a lot more
>> interested in getting bindings upstreamed ASAP (so we can start
>> guaranteeing no backwards-compat breaks, which is important to avoid
>> outright breakage) than I am in trying to be exhaustive up front with
>> device instances. It's perfectly fine to say that users have to upgrade
>> both their DTs and kernels to get newer hardware support, on these
>> platforms.
> 
> It's also a very common process for SoCs - almost no one writes the
> DT first and then writes the drivers. You always see on the mailing
> list series of patches that add a driver for some bit of hardware,
> along with patches adding the DT binding and the DT description.
> 
> I don't think you're doing anything different here to what is common
> practice within the mainline kernel community with this approach.
> 
> The exception to that is when adding a driver for feature X in a SoC,
> it's common to add all instances of X to the dtsi with ``status =
> "disabled"'' and only enable the appropriate blocks on platforms that
> need it.
> 
> So, for example, if a SoC has three network interfaces, all of them
> identical, when adding a network driver and the bindings for the
> network hardware, one would add all three to the SoC description
> whether or not the platform one was working with makes use of them.

Right, and that makes sense for SoCs that third parties can incorporate
into any arbitrary platform, and which have documentation listing those
instances, because then board-level DTs can just enable the ones they need.

But here we're working with SoCs where the only reference for what
hardware exists *is* Apple's DT, and it only lists hardware that is in
use on existing platforms. So there is no sane way for us to know what
hardware exists, except what we can infer from reading the tea leaves
(e.g. you can tell how many UARTs there are from the power domains and
leaked schematics, but this doesn't extend to everything else). For
example, we also know that these SoCs have a second Neural Engine that
Apple have disabled on all shipping platforms, that the t600x series has
an unused USB3 controller, etc... but there's no way we can safely
describe these things in the DT if we can't test that they work and we
don't have documentation even telling us how they're supposed to work :)

> In the case of gpio-macsmc, how would we later add support for the
> slave PMU GPIOs, given that these use keys "gpXX" rather than "gPxx"?
> How do we tell the gpio-macsmc code to use a different set of keys?
> Should DT describe the key "prefix" (in other words "gp" vs "gP"),
> or should it describe it some other way. What if Apple decides to
> instantiate another GPIO controller in a later platform with a
> different prefix, could that be accomodated without breaking any
> solution we come up today?
> 
> Maybe the solution to this would be to describe the key prefix in DT
> as that's effectively its "reg". Or maybe we use "reg" to describe
> it somehow (which is value of the key, which seems to have an
> "address" like quality to it?)
> 
> We don't have to implement code for this now, we just need to get a
> reasonably correct DT binding for the gpio controller.

I agree that this is something to think about (I was about to reply on
the subject).

I can think of two ways: using `reg` for the key name, but that feels
icky since it's ASCII and not *really* a register number/address, or
something like this:

gpio@0 {
	apple,smc-key-base = "gP00";
	...
}

gpio@1 {
	apple,smc-key-base = "gp00";
	...
}

But this ties back to the device enumeration too, since right now the DT
does not drive that (we'd have to add the subdevice to the mfd subdevice
list somehow anyway, if we don't switch to compatibles).

I'd love to hear Rob's opinion on this one, and also whether the
existing Linux and OpenBSD code would currently find gpio@0 {} instead
of gpio {} for backwards compat.

- Hector