public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] fdt: Allow fdt_translate_address() to work with buses
Date: Mon, 4 Jan 2016 13:15:53 -0700	[thread overview]
Message-ID: <568AD2F9.2050903@wwwdotorg.org> (raw)
In-Reply-To: <1451862280-15245-1-git-send-email-sjg@chromium.org>

On 01/03/2016 04:04 PM, Simon Glass wrote:
> It is common for I2C and SPI buses to have a single-cell address and a size
> of 0. These produce a warning at present. For example on snow:
>
> __of_translate_address: Bad cell count for gpc4
> __of_translate_address: Bad cell count for gpx0
> __of_translate_address: Bad cell count for gpv2
> __of_translate_address: Bad cell count for gpv4
>
> One of the nodes in question looks like this in part:
>
> 	pinctrl_2: pinctrl at 10d10000 {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		gpv2: gpv2 {
> 			reg = <0x060>;
> 		};
> 		gpv4: gpv4 {
> 			reg = <0xc0>;
> 		};
> 	};
>
> This is clearly valid so it looks like the conversion to use
> fdt_translate_address() in dev_get_addr() is not currently a good move.

To disable that, why not simply turn off CONFIG_OF_TRANSLATE on the 
affected platforms? That's precisely why that config option was 
introduced when the call to fdt_translate_address() was added to 
dev_get_addr()?

That would prevent this patch from affecting platforms that don't 
trigger this issue, this leaving the valid check in place.

> Przemyslaw Marczak sent three patches to resolve this for exynos boards:
>
> https://patchwork.ozlabs.org/patch/557008/
> https://patchwork.ozlabs.org/patch/557010/
> https://patchwork.ozlabs.org/patch/557009/
>
> But this involves creating a new function, and everyone will need to know
> when to use which one. Also the problem may affect other boards.

I suggest adding an extra parameter to dev_get_addr() (or whatever calls 
it) that indicates the root of the address space. The check on 
#size-cells should be skipped for that one node (or level of 
translation) but enabled for all other levels. This way, there would be 
no need for anyone to choose between functions; there'd only be one. 
Most cases (i.e. translation of MMIO addresses) would simply pass 0 as 
the extra parameter (for the root node), but in special cases where it's 
known translation is not expected to reach the root MMIO space (e.g. 
I2C, SPI controllers), the controller node would be passed in.

  parent reply	other threads:[~2016-01-04 20:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-03 23:04 [U-Boot] [PATCH] fdt: Allow fdt_translate_address() to work with buses Simon Glass
2016-01-04  8:35 ` Przemyslaw Marczak
2016-01-04 20:15 ` Stephen Warren [this message]
2016-01-05  1:00   ` Simon Glass
2016-01-05 15:47     ` Przemyslaw Marczak
2016-01-05 17:26       ` Stephen Warren
2016-01-07 11:43         ` Przemyslaw Marczak
2016-01-07 18:22           ` Stephen Warren
2016-01-05 17:15     ` Stephen Warren

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=568AD2F9.2050903@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=u-boot@lists.denx.de \
    /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