On Thu, Apr 08, 2021 at 04:14:07AM +1200, Simon Glass wrote: > Hi, > > On Thu, 8 Apr 2021 at 02:42, Rob Herring <r...@kernel.org> wrote: > > > > On Tue, Apr 6, 2021 at 4:53 PM Dario Binacchi <dario...@libero.it> wrote: > > > > > > > > > > Il 06/04/2021 16:26 Rob Herring <r...@kernel.org> ha scritto: > > > > > > > > > > > > On Tue, Mar 16, 2021 at 8:26 PM Bin Meng <bmeng...@gmail.com> wrote: > > > > > > > > > > Hi Dario, > > > > > > > > > > On Wed, Mar 17, 2021 at 4:57 AM Dario Binacchi <dario...@libero.it> > > > > > wrote: > > > > > > > > > > > > Hi Bin, > > > > > > > > > > > > > Il 16/03/2021 02:28 Bin Meng <bmeng...@gmail.com> ha scritto: > > > > > > > > > > > > > > > > > > > > > Hi Dario, > > > > > > > > > > > > > > On Tue, Mar 16, 2021 at 6:49 AM Dario Binacchi > > > > > > > <dario...@libero.it> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > Il 15/03/2021 19:23 Simon Glass <s...@chromium.org> ha > > > > > > > > > scritto: > > > > > > > > > > > > > > > > > > > > > > > > > > > +Tom Rini too > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, 16 Mar 2021 at 03:48, Bin Meng <bmeng...@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > +Dario Binacchi > > > > > > > > > > > > > > > > > > > > On Mon, Mar 15, 2021 at 3:11 PM Simon Glass > > > > > > > > > > <s...@chromium.org> wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Bin, > > > > > > > > > > > > > > > > > > > > > > On Wed, 3 Mar 2021 at 14:54, Simon Glass > > > > > > > > > > > <s...@chromium.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Fri, 26 Feb 2021 at 00:36, Bin Meng > > > > > > > > > > > > <bmeng...@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > The implementation of of_translate_one() was taken > > > > > > > > > > > > > from the one in > > > > > > > > > > > > > Linux kernel drivers/of/address.c, and the Linux one > > > > > > > > > > > > > added a quirk > > > > > > > > > > > > > for Apple Macs that don't have the <ranges> property > > > > > > > > > > > > > in the parent > > > > > > > > > > > > > node. Since U-Boot does not support Apple Macs, > > > > > > > > > > > > > remove the comment > > > > > > > > > > > > > block and adhere to the spec to abort the translation. > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Bin Meng <bmeng...@gmail.com> > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > > > > > drivers/core/of_addr.c | 24 ++++++------------------ > > > > > > > > > > > > > 1 file changed, 6 insertions(+), 18 deletions(-) > > > > > > > > > > > > > > > > > > > > > > > > Reviewed-by: Simon Glass <s...@chromium.org> > > > > > > > > > > > > > > > > > > > > > > Unfortunately this seems to cause a test failure for > > > > > > > > > > > ut_dm_fdt_translation. Can you please take a look? > > > > > > > > > > > > > > > > > > > > It seems the no "ranges" property was intentionally removed > > > > > > > > > > by the > > > > > > > > > > following commit: > > > > > > > > > > > > > > > > > > > > commit d64b9cdcd475eb7f07b49741ded87e24dae4a5fc > > > > > > > > > > Author: Dario Binacchi <dario...@libero.it> > > > > > > > > > > Date: Wed Dec 30 00:16:21 2020 +0100 > > > > > > > > > > > > > > > > > > > > fdt: translate address if #size-cells = <0> > > > > > > > > > > > > > > > > > > > > The __of_translate_address routine translates an > > > > > > > > > > address from the > > > > > > > > > > device tree into a CPU physical address. A note in the > > > > > > > > > > description of > > > > > > > > > > the routine explains that the crossing of any level with > > > > > > > > > > since inherited from IBM. This does not happen for > > > > > > > > > > Texas Instruments, or > > > > > > > > > > at least for the beaglebone device tree. Without this > > > > > > > > > > patch, in fact, > > > > > > > > > > the translation into physical addresses of the > > > > > > > > > > registers contained in the > > > > > > > > > > am33xx-clocks.dtsi nodes would not be possible. They > > > > > > > > > > all have a parent > > > > > > > > > > with #size-cells = <0>. > > > > > > > > > > > > > > > > > > > > It looks the commit was needed for beaglebone board. > > > > > > > > > > > > > > > > > > > > Dario, could you please comment on why U-Boot needs to done > > > > > > > > > > like this, > > > > > > > > > > while Linux kernel has this check? Is the beaglebone board > > > > > > > > > > not working > > > > > > > > > > in Linux? > > > > > > > > > > > > > > > > > > > > > > > > > > Beaglebone is working in Linux, but I think Linux walks the > > > > > > > > device tree less > > > > > > > > fully than u-boot. > > > > > > > > I was surprised by the address translation error when > > > > > > > > traversing nodes with > > > > > > > > size cells 0. And for this reason I added the > > > > > > > > CONFIG_OF_TRANSLATE_ZERO_SIZE_CELLS > > > > > > > > symbol to fix the issue, not enabled by default, thus making > > > > > > > > the change backwards > > > > > > > > compatible. > > > > > > > > > > > > > > Could you please prepare a patch against upstream Linux kernel, so > > > > > > > that U-Boot can be in sync with it? > > > > > > > > I've replied on that patch... > > > > > > > > > > > > > > > > With pleasure. But how do I justify the patch since it doesn't fix > > > > > > any bugs? > > > > > > Can I refer to the patches developed for U-boot? > > > > > > > > > > Good question :) > > > > > > > > > > If Linux does not have any issue, maybe U-Boot's fix is questionable? > > > > > > > > IMO, yes it is. Simply put, 'ranges' must be present to be > > > > translatable. Using '#size-cells == 0' is at least questionable, but > > > > could maybe be supported (depends what happens with non-empty > > > > 'ranges'). > > > > > > > > The 'fix' makes every 'reg' translatable. Give the function an I2C > > > > address 'reg' and you'll get back 'I2C controller address + I2C > > > > address'. > > > > > > IMHO this could mean that using size-cells = <0> in the prcm_clocks and > > > scm_clocks nodes is perhaps not quite correct. We need an address > > > translation > > > and if possible, better without having to develop code to handle this > > > special > > > case. > > > > I agree as with a size of 0 there's an implicit assumption as to what > > the size of the registers are. It's 4 bytes in your case, but what if > > I have 2 byte registers? > > > > > > > > With the following changes in the am33xx-l4.dtsi and am33xx-clocks.dtsi > > > files we > > > get an address translation that does not require special case handling > > > code and > > > the device tree would not be an exception to what we usually see. I have > > > tested > > > these changes in the kernel and they seem to work. > > > > That is the correct change, but compatibility is the issue here. > > Just to dig into this a bit, at least within U-Boot the DT is captive, > so we should be able to make this change. If there is a quirk needed, > then perhaps it can be added with a property to enable it. > > Can you give a few more details about the compatibility problem?
These are files that in U-Boot we keep untouched and in-sync with the kernel. We might be able to whack something in to -u-boot.dtsi but that's not ideal when there's an actual problem being exposed that needs fixing. -- Tom
signature.asc
Description: PGP signature
