From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mario Six Date: Thu, 1 Mar 2018 08:26:58 +0100 Subject: [U-Boot] Regression: gpio: pca953x_gpio: Make live-tree compatible In-Reply-To: References: Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de (Adding Simon, because of DM) Hi Martin, On Wed, Feb 28, 2018 at 7:27 PM, Martin Fuzzey wrote: > Hi, > > since this commit: > > commit f62ca2cd2ab8171f603960da9203ceb6ee8a1efd > Author: Mario Six > Date: Mon Jan 15 11:07:45 2018 +0100 > > gpio: pca953x_gpio: Make live-tree compatible > > > I am getting the error message "__of_translate_address: Bad cell count for > pcal6524 at 1" > > I don't think the patch > > - addr = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), "reg", 0); > + addr = dev_read_addr(dev); > > is correct since it replaces a simple lookup of the value "reg" property > with an attempted translation to a CPU address (which fails for I2C) > I see the issue on one of our boards as well, but the weird thing is that the translation should not happen: the I2C bus has no ranges property, which should, according to the DeviceTree specification, cause the translation routine to not attempt a translation (because a missing ranges property implies that the device's address space is not translatable). I'll take a look at the code; maybe it's a Linux-specific exception that was imported with the code that I was not aware of. The dev_read_addr() is the go-to function for drivers to get device addresses (and have all issues like translation and the like taken care of automatically), so it would be nice to keep it in my opinion. Hence, fixing the underlying issue is probably preferable. > This does not cause the driver probe to fail since dev_read_addr() returns > FDT_ADDR_T_NONE (== -1) and is followed by > > if (addr == 0) > return -ENODEV; > > info->addr = addr; > > Although addr is stored in the driver data the only thing it is used for is > to build the bank name > > snprintf(name, sizeof(name), "gpio@%x_", info->addr); > > I'm not even sure that using the value of the "reg" property as before is > really a good idea - what happens if we have multiple chips with same > address on seperate busses? > We have that exact case on some of our boards. It works, but it's pretty annoying if you want to use the gpio command: When setting values of GPIOs, it only ever affects the first device the command comes across. I have a potential solution for that, see below. > Maybe it would be better to use the DT node name? But would that break > existing configurations by renaming the device? I have a patch I use on our boards that looks for a "label" string property in the DT, which is then used for the bank name if it's there, and falls back to the "gpio@%x_" in the case it's not there. If you think it's useful, I could send the patch to the mailing list? > > > Regards, > > Martin > Best regards, Mario