> -----Original Message-----
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: 14 September 2016 22:32
> To: linux-arm-ker...@lists.infradead.org
> Cc: Yuanzhichang; devicet...@vger.kernel.org;
> lorenzo.pieral...@arm.com; Gabriele Paoloni; miny...@acm.org;
> gre...@linuxfoundation.org; b...@kernel.crashing.org; John Garry;
> will.dea...@arm.com; firstname.lastname@example.org; xuwei (O); Linuxarm;
> linux-ser...@vger.kernel.org; linux-...@vger.kernel.org;
> zourongr...@gmail.com; liviu.du...@arm.com; kant...@163.com;
> Subject: Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on
> On Wednesday, September 14, 2016 10:50:44 PM CEST zhichang.yuan wrote:
> > On 2016/9/14 20:33, Arnd Bergmann wrote:
> > > On Wednesday, September 14, 2016 8:15:52 PM CEST Zhichang Yuan
> > >
> > >> +Required properties:
> > >> +- compatible: should be "hisilicon,low-pin-count"
> > >> +- #address-cells: must be 2 which stick to the ISA/EISA binding
> > >> +- #size-cells: must be 1 which stick to the ISA/EISA binding doc.
> > >> +- reg: base address and length of the register set for the
> > >> +- ranges: define a 1:1 mapping between the I/O space of the child
> device and
> > >> + the parent.
> > >
> > > Do we still need the "ranges" here? The property in your example
> > > wrong.
> > I think "ranges" is needed.
> > without this, of_translate_address --> __of_translate_address -->
> of_translate_one will fail when translating the child's IO resource.
> > >
> > >> + ranges = <0x01 0xe4 0x0 0xe4 0x1000>;
> > >
> > > You translate I/O port 0x00e4 through 0x10e4 to CPU address 0x0e4?
> > The hip06 LPC is defined as isa type.
> > So, 0x01 0xe4 is the local IO address of 0xe4. With this ranges, 0xe4
> of child will be 1:1 mapped as 0xe4.
> > It means no translation.
> No, "no translation" would be leaving out the ranges, we should
> fix the code so it handles this case according to the specification
> of the ISA DT binding, rather than adding an incorrect ranges
> property to make it work with the incorrect Linux implementation.
> of_translate_address() should fail here, and whichever code calls
> it should try something else, possibly something we have to
> implement that can return the correct IORESOURCE_* type.
>From <<3.1.1. Open Firmware Properties for Bus Nodes>> in
"There shall be an entry in the "ranges" property for each
of the Memory and/or I/O spaces if that address space is
mapped through the bridge."
It seems to me that it is ok to have 1:1 address mapping and that
therefore of_translate_address() should fail if "ranges" is not
This is also explained quite well in
what do you think?
> > >
> > > I don't get this part. The bus driver should not care what its
> > > children are, just register and PIO ranges that the bus can handle
> > > in theory, i.e. from 0x000 to 0xfff.
> > Just as we discussed in V2, the legacy PIO range is specific to some
> > device, such as for ipmi bt, 0xe4 - 0xe7 will be populated.
> > I don't want to occupy a larger PIO range in which only small part
> > are used by our LPC. At this moment, two PIO ranges are using
> > through the device property configuration, 0xe4-0xe7, 0x2f8-0x2ff.
> > If we configure 0-0x1000 for the LPC to cover those two ranges, most
> > PIO are wasted and other PIO device on other buses lose the chance to
> > use the PIO below 0x1000.
> > Otherwise, PIO conflict will happen. So, My idea is only occupied
> > the PIO ranges which are really needed for the children.
> The only thing it can realistically conflict with would be another
> LPC bus behind on a PCI host bridge. On ARM64, all regular PCI
> devices that have IORESOURCE_IO ports are intentionally moved
> to (bus) port numbers above 0x1000.
> > And there are probably multiple child devices under LPC, the global
> arm64_extio_ops only can cover one PIO range. It is fortunate only ipmi
> driver can not support I/O
> > operation registering, serial driver has serial_in/serial_out to
> > be registered. So, only the PIO range for ipmi device is stored
> > in arm64_extio_ops and the indirect-IO
> > works well for ipmi device.
> You should not do that in the serial driver, please just use the
> normal 8250 driver that works fine once you handle the entire
> port range.