Re: [PATCH 2/3] dt-bindings: serial: tegra-tcu: Document "reg" property

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]

On Sat, Mar 27, 2021 at 12:49:25PM -0600, Rob Herring wrote:
> On Fri, Mar 26, 2021 at 06:46:40PM +0100, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > In order to support early console support with the Tegra TCU, add an
> > optional "reg" property that points to the TX mailbox.
> 
> This will mean the same address in in DT twice which we try to avoid, 
> right? I guess it's fine, we could drop it if we ever enforce that. I'm 
> sure there's worse abuses with duplicates than this.

It doesn't end up being the exact same address, but it'll be an address
that is part of the I/O memory region defined by the HSP block that
provides the mailbox. In practice this isn't going to cause a problem
because by the time the mailbox address is being used, the early console
will have been replaced by the proper TCU console. But yeah, I'm not
particularly thrilled by this, either.

I had spent a bit of time trying to code up a setup function that would
parse the necessary information from DT, but this unfortunately required
a bunch of ->setup() callbacks to be changed because we don't currently
pass in the FDT node offset, which we'd need in order for this to work.
An additional negative side-effect is that we'd need to duplicate a
bunch of logic to determine the offset to add onto the HSP base address
and that's something that depends on the particular configuration of the
HSP instance, which means lots of hard-coded conditionals.

All of that added up and at some point this slight abuse seemed like the
lesser evil.

> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  .../devicetree/bindings/serial/nvidia,tegra194-tcu.yaml     | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.yaml b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.yaml
> > index 0a321658ccb5..8c9ed7cfaa52 100644
> > --- a/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.yaml
> > +++ b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.yaml
> > @@ -24,6 +24,9 @@ properties:
> >    compatible:
> >      const: nvidia,tegra194-tcu
> >  
> > +  reg:
> > +    maxItems: 1
> > +
> >    mbox-names:
> >      items:
> >        - const: rx
> > @@ -48,8 +51,9 @@ examples:
> >    - |
> >      #include <dt-bindings/mailbox/tegra186-hsp.h>
> >  
> > -    tcu: tcu {
> > +    tcu: tcu@c168000 {
> 
> While you're here:
> 
> serial@...

Okay, will do. It's not a very traditional serial port, so that might
perhaps be confusing, but we're using it like a serial port, so for all
intents and purposes this really is a serial port, so renaming makes
sense, I think.

Thierry

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