From: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>,
Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH] checks: Handle #dma-address-cells and #dma-size-cells
Date: Mon, 14 Nov 2022 11:34:06 +0100 [thread overview]
Message-ID: <Y3IZnpL3Q3STEaLs@orome> (raw)
In-Reply-To: <CAL_JsqLV2ZHLJ=14zf9zNfq+S+Rs09EYmZrNHsvdhbmvvehj6g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 7677 bytes --]
On Fri, Nov 11, 2022 at 11:01:58AM -0600, Rob Herring wrote:
> On Fri, Nov 11, 2022 at 5:47 AM Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> >
> > The #dma-address-cells and #dma-size-cells properties can be used to
> > override their non-DMA counterparts (#address-cells and #size-cells)
> > for cases where the control bus (defined by the "reg" and "ranges"
> > properties) has different addressing requirements from the DMA bus.
> >
> > The "dma-ranges" property needs to be sized depending on these DMA
> > bus properties rather than the control bus properties, so adjust the
> > check accordingly.
>
> I assume I'll be seeing a spec and schema addition too.
Yeah, I was looking around to see where else we may need changes. I had
looked at dt-schema but couldn't find a good place to add them since
#address-cells and #size-cells seem to be mostly handled in the library
code rather than in the json-schema definitions. So if you could provide
some pointers as to how you think this should be added, that'd be great.
I can look at writing an update to the spec, but to be frank could use
some guidance on that as well.
> I imagine David is wondering where this is coming from given these are
> new properties, not existing ones that the checks just didn't handle.
Right, I should've provided a bit more background on this. Evidently the
commit message was not enough.
Thierry
> > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> > checks.c | 48 +++++++++++++++++++++++++++++++++++++-----------
> > dtc.h | 1 +
> > 2 files changed, 38 insertions(+), 11 deletions(-)
> >
> > diff --git a/checks.c b/checks.c
> > index 9f31d2607182..ee13dce03483 100644
> > --- a/checks.c
> > +++ b/checks.c
> > @@ -743,6 +743,17 @@ static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
> > prop = get_property(node, "#size-cells");
> > if (prop)
> > node->size_cells = propval_cell(prop);
> > +
> > + node->dma_addr_cells = -1;
> > + node->dma_size_cells = -1;
> > +
> > + prop = get_property(node, "#dma-address-cells");
> > + if (prop)
> > + node->dma_addr_cells = propval_cell(prop);
>
> else
> node->dma_addr_cells = node->addr_cells;
>
> > +
> > + prop = get_property(node, "#dma-size-cells");
> > + if (prop)
> > + node->dma_size_cells = propval_cell(prop);
> > }
> > WARNING(addr_size_cells, fixup_addr_size_cells, NULL,
> > &address_cells_is_cell, &size_cells_is_cell);
> > @@ -751,6 +762,10 @@ WARNING(addr_size_cells, fixup_addr_size_cells, NULL,
> > (((n)->addr_cells == -1) ? 2 : (n)->addr_cells)
> > #define node_size_cells(n) \
> > (((n)->size_cells == -1) ? 1 : (n)->size_cells)
> > +#define node_dma_addr_cells(n) \
> > + (((n)->dma_addr_cells == -1) ? node_addr_cells(n) : (n)->dma_addr_cells)
> > +#define node_dma_size_cells(n) \
> > + (((n)->dma_size_cells == -1) ? node_size_cells(n) : (n)->dma_size_cells)
>
> Then these can be just "(n)->dma_size_cells".
>
> But wait, what about the default sizes? Those have been a dtc warning
> since longer than I've run dtc. The defaults don't even agree with
> Linux depending on the architecture. Certainly anyone using these new
> properties must not be relying on defaults.
>
> >
> > static void check_reg_format(struct check *c, struct dt_info *dti,
> > struct node *node)
> > @@ -787,6 +802,7 @@ static void check_ranges_format(struct check *c, struct dt_info *dti,
> > struct property *prop;
> > int c_addr_cells, p_addr_cells, c_size_cells, p_size_cells, entrylen;
> > const char *ranges = c->data;
> > + const char *prefix;
> >
> > prop = get_property(node, ranges);
> > if (!prop)
> > @@ -798,28 +814,38 @@ static void check_ranges_format(struct check *c, struct dt_info *dti,
> > return;
> > }
> >
> > - p_addr_cells = node_addr_cells(node->parent);
> > - p_size_cells = node_size_cells(node->parent);
> > - c_addr_cells = node_addr_cells(node);
> > - c_size_cells = node_size_cells(node);
> > + if (strcmp(ranges, "dma-ranges") == 0) {
> > + p_addr_cells = node_dma_addr_cells(node->parent);
> > + p_size_cells = node_dma_size_cells(node->parent);
> > + c_addr_cells = node_dma_addr_cells(node);
> > + c_size_cells = node_dma_size_cells(node);
> > + prefix = "dma-";
> > + } else {
> > + p_addr_cells = node_addr_cells(node->parent);
> > + p_size_cells = node_size_cells(node->parent);
> > + c_addr_cells = node_addr_cells(node);
> > + c_size_cells = node_size_cells(node);
> > + prefix = "";
> > + }
> > +
> > entrylen = (p_addr_cells + c_addr_cells + c_size_cells) * sizeof(cell_t);
> >
> > if (prop->val.len == 0) {
> > if (p_addr_cells != c_addr_cells)
> > FAIL_PROP(c, dti, node, prop, "empty \"%s\" property but its "
> > - "#address-cells (%d) differs from %s (%d)",
> > - ranges, c_addr_cells, node->parent->fullpath,
> > + "#%saddress-cells (%d) differs from %s (%d)",
> > + ranges, prefix, c_addr_cells, node->parent->fullpath,
>
> This is going to misleadingly print '#dma-address-cells' in cases that
> actually have '#address-cells'. It's probably easiest if you say 'DMA?
> address cells' rather than using the property name.
>
> > p_addr_cells);
> > if (p_size_cells != c_size_cells)
> > FAIL_PROP(c, dti, node, prop, "empty \"%s\" property but its "
> > - "#size-cells (%d) differs from %s (%d)",
> > - ranges, c_size_cells, node->parent->fullpath,
> > + "#%ssize-cells (%d) differs from %s (%d)",
> > + ranges, prefix, c_size_cells, node->parent->fullpath,
> > p_size_cells);
> > } else if (!is_multiple_of(prop->val.len, entrylen)) {
> > FAIL_PROP(c, dti, node, prop, "\"%s\" property has invalid length (%d bytes) "
> > - "(parent #address-cells == %d, child #address-cells == %d, "
> > - "#size-cells == %d)", ranges, prop->val.len,
> > - p_addr_cells, c_addr_cells, c_size_cells);
> > + "(parent #%saddress-cells == %d, child #%saddress-cells == %d, "
> > + "#size-cells == %d)", ranges, prop->val.len, prefix,
> > + p_addr_cells, prefix, c_addr_cells, c_size_cells);
> > }
> > }
> > WARNING(ranges_format, check_ranges_format, "ranges", &addr_size_cells);
> > diff --git a/dtc.h b/dtc.h
> > index 0a1f54991026..3d2ef7f3616f 100644
> > --- a/dtc.h
> > +++ b/dtc.h
> > @@ -228,6 +228,7 @@ struct node {
> >
> > cell_t phandle;
> > int addr_cells, size_cells;
> > + int dma_addr_cells, dma_size_cells;
> >
> > struct label *labels;
> > const struct bus_type *bus;
> > --
> > 2.38.1
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2022-11-14 10:34 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-11 11:47 [PATCH] checks: Handle #dma-address-cells and #dma-size-cells Thierry Reding
[not found] ` <20221111114728.462767-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2022-11-11 17:01 ` Rob Herring
[not found] ` <CAL_JsqLV2ZHLJ=14zf9zNfq+S+Rs09EYmZrNHsvdhbmvvehj6g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-11-13 5:07 ` David Gibson
2022-11-14 10:34 ` Thierry Reding [this message]
2022-11-14 14:27 ` Thierry Reding
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y3IZnpL3Q3STEaLs@orome \
--to=thierry.reding-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
--cc=david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org \
--cc=devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
--cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).