diff mbox

[V3,2/4] ARM64 LPC: LPC driver implementation on Hip06

Message ID 1473855354-150093-3-git-send-email-yuanzhichang@hisilicon.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Zhichang Yuan Sept. 14, 2016, 12:15 p.m. UTC
From: "zhichang.yuan" <yuanzhichang@hisilicon.com>

On Hip06, the accesses to LPC peripherals work in an indirect way. A
corresponding LPC driver configure some registers in LPC master at first, then
the real accesses on LPC slave devices are finished by the LPC master, which
is transparent to LPC driver.
This patch implement the relevant driver for Hip06 LPC. Cooperating with
indirect-IO, ipmi messages is in service without any changes on ipmi driver.

Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
---
 .../arm/hisilicon/hisilicon-low-pin-count.txt      |  35 ++
 drivers/bus/Kconfig                                |   8 +
 drivers/bus/Makefile                               |   2 +
 drivers/bus/hisi_lpc.c                             | 653 +++++++++++++++++++++
 drivers/of/address.c                               |   9 +
 5 files changed, 707 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
 create mode 100644 drivers/bus/hisi_lpc.c

Comments

Arnd Bergmann Sept. 14, 2016, 12:33 p.m. UTC | #1
On Wednesday, September 14, 2016 8:15:52 PM CEST Zhichang Yuan wrote:

> +Required properties:
> +- compatible: should be "hisilicon,low-pin-count"
> +- #address-cells: must be 2 which stick to the ISA/EISA binding doc.
> +- #size-cells: must be 1 which stick to the ISA/EISA binding doc.
> +- reg: base address and length of the register set for the device.
> +- 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 seems
wrong.

> +	ranges = <0x01 0xe4 0x0 0xe4 0x1000>;

You translate I/O port 0x00e4 through 0x10e4 to CPU address 0x0e4?

> +/**
> + * hisilpc_children_map_sysio - setup the mapping between system Io and
> + *			physical IO
> + *
> + * @child: the device whose IO is handling
> + * @data: some device specific data. For ACPI device, should be NULL.
> + *
> + * Returns >=0 means the mapping is successfully created;
> + * others mean some failures.
> + */
> +static int hisilpc_children_map_sysio(struct device * child, void * data)
> +{
> +	struct resource *iores;
> +	unsigned long cpuio;
> +	struct extio_ops *opsnode;
> +	int ret;
> +	struct hisilpc_dev *lpcdev;
> +
> +	if (!child || !child->parent)
> +		return -EINVAL;
> +
> +	iores = platform_get_resource_byname(to_platform_device(child),
> +					IORESOURCE_IO, "dev_io");
> +	if (!iores)
> +		return -ENODEV;
> +
> +	/*
> +	 * can not use devm_kzalloc to allocate slab for child before its driver
> +	 * start probing. Here allocate the slab with the name of parent.
> +	 */
> +	opsnode = devm_kzalloc(child->parent, sizeof(*opsnode), GFP_KERNEL);
> +	if (!opsnode)
> +		return -ENOMEM;
> +
> +	cpuio = data ? *((unsigned long *)data) : 0;
> +
> +	opsnode->start = iores->start;
> +	opsnode->end = iores->end;
> +	opsnode->ptoffset = cpuio ? (cpuio - iores->start) : 0;
> +
> +	dev_info(child, "map sys port[%lx - %lx] offset=0x%lx",
> +				(unsigned long)iores->start,
> +				(unsigned long)iores->end,
> +				opsnode->ptoffset);
> +
> +	opsnode->pfin = hisilpc_comm_inb;
> +	opsnode->pfout = hisilpc_comm_outb;
> +
> +	lpcdev = platform_get_drvdata(to_platform_device(child->parent));
> +	opsnode->devpara = lpcdev;
> +
> +	/* only apply indirect-IO to ipmi child device */

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.

	Arnd

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
kernel test robot Sept. 14, 2016, 2:09 p.m. UTC | #2
Hi zhichang.yuan,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.8-rc6 next-20160914]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Zhichang-Yuan/ARM64-LPC-legacy-ISA-I-O-support/20160914-202858
config: openrisc-or1ksim_defconfig (attached as .config)
compiler: or32-linux-gcc (GCC) 4.5.1-or32-1.0rc1
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=openrisc 

All errors (new ones prefixed by >>):

   drivers/of/address.c: In function '__of_address_to_resource':
>> drivers/of/address.c:702:5: error: implicit declaration of function 'of_bus_pci_match'

vim +/of_bus_pci_match +702 drivers/of/address.c

   696				return -EINVAL;
   697			/*
   698			 * special processing for non-pci device gurantee the linux start pio
   699			 * is not ZERO. Otherwise, some drivers' initialization will fail.
   700			 */
   701			if (!port && (!IS_ENABLED(CONFIG_OF_ADDRESS_PCI) ||
 > 702					!of_bus_pci_match(dev)))
   703				port += 1;
   704	
   705			r->start = port;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Zhichang Yuan Sept. 14, 2016, 2:50 p.m. UTC | #3
On 2016/9/14 20:33, Arnd Bergmann wrote:
> On Wednesday, September 14, 2016 8:15:52 PM CEST Zhichang Yuan wrote:
> 
>> +Required properties:
>> +- compatible: should be "hisilicon,low-pin-count"
>> +- #address-cells: must be 2 which stick to the ISA/EISA binding doc.
>> +- #size-cells: must be 1 which stick to the ISA/EISA binding doc.
>> +- reg: base address and length of the register set for the device.
>> +- 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 seems
> 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.

> 
>> +/**
>> + * hisilpc_children_map_sysio - setup the mapping between system Io and
>> + *			physical IO
>> + *
>> + * @child: the device whose IO is handling
>> + * @data: some device specific data. For ACPI device, should be NULL.
>> + *
>> + * Returns >=0 means the mapping is successfully created;
>> + * others mean some failures.
>> + */
>> +static int hisilpc_children_map_sysio(struct device * child, void * data)
>> +{
>> +	struct resource *iores;
>> +	unsigned long cpuio;
>> +	struct extio_ops *opsnode;
>> +	int ret;
>> +	struct hisilpc_dev *lpcdev;
>> +
>> +	if (!child || !child->parent)
>> +		return -EINVAL;
>> +
>> +	iores = platform_get_resource_byname(to_platform_device(child),
>> +					IORESOURCE_IO, "dev_io");
>> +	if (!iores)
>> +		return -ENODEV;
>> +
>> +	/*
>> +	 * can not use devm_kzalloc to allocate slab for child before its driver
>> +	 * start probing. Here allocate the slab with the name of parent.
>> +	 */
>> +	opsnode = devm_kzalloc(child->parent, sizeof(*opsnode), GFP_KERNEL);
>> +	if (!opsnode)
>> +		return -ENOMEM;
>> +
>> +	cpuio = data ? *((unsigned long *)data) : 0;
>> +
>> +	opsnode->start = iores->start;
>> +	opsnode->end = iores->end;
>> +	opsnode->ptoffset = cpuio ? (cpuio - iores->start) : 0;
>> +
>> +	dev_info(child, "map sys port[%lx - %lx] offset=0x%lx",
>> +				(unsigned long)iores->start,
>> +				(unsigned long)iores->end,
>> +				opsnode->ptoffset);
>> +
>> +	opsnode->pfin = hisilpc_comm_inb;
>> +	opsnode->pfout = hisilpc_comm_outb;
>> +
>> +	lpcdev = platform_get_drvdata(to_platform_device(child->parent));
>> +	opsnode->devpara = lpcdev;
>> +
>> +	/* only apply indirect-IO to ipmi child device */
> 
> 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 PIOs 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.

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.

If we think it is nearly no chance that LPC PIO range conflict with other buses, we can allocate 0xe4 - 0x2ff to LPC and store it to arm64_extio_ops. In this case,
the special processing for ipmi device is not needed.

Best,
Zhichang

> 
> 	Arnd
> 
> 
> .
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Sept. 14, 2016, 9:32 p.m. UTC | #4
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 wrote:
> > 
> >> +Required properties:
> >> +- compatible: should be "hisilicon,low-pin-count"
> >> +- #address-cells: must be 2 which stick to the ISA/EISA binding doc.
> >> +- #size-cells: must be 1 which stick to the ISA/EISA binding doc.
> >> +- reg: base address and length of the register set for the device.
> >> +- 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 seems
> > 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.

> > 
> > 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 PIOs
> 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.
 

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gabriele Paoloni Sept. 15, 2016, 8:02 a.m. UTC | #5
Hi Arnd

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 14 September 2016 22:32
> To: linux-arm-kernel@lists.infradead.org
> Cc: Yuanzhichang; devicetree@vger.kernel.org;
> lorenzo.pieralisi@arm.com; Gabriele Paoloni; minyard@acm.org;
> gregkh@linuxfoundation.org; benh@kernel.crashing.org; John Garry;
> will.deacon@arm.com; linux-kernel@vger.kernel.org; xuwei (O); Linuxarm;
> linux-serial@vger.kernel.org; linux-pci@vger.kernel.org;
> zourongrong@gmail.com; liviu.dudau@arm.com; kantyzc@163.com;
> zhichang.yuan02@gmail.com
> Subject: Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on
> Hip06
> 
> 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
> wrote:
> > >
> > >> +Required properties:
> > >> +- compatible: should be "hisilicon,low-pin-count"
> > >> +- #address-cells: must be 2 which stick to the ISA/EISA binding
> doc.
> > >> +- #size-cells: must be 1 which stick to the ISA/EISA binding doc.
> > >> +- reg: base address and length of the register set for the
> device.
> > >> +- 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
> seems
> > > 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 
http://www.firmware.org/1275/bindings/isa/isa0_4d.ps

I quote:
"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
present.

This is also explained quite well in
http://lxr.free-electrons.com/source/drivers/of/address.c#L490

what do you think?

Thanks

Gab

> 
> > >
> > > 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
> PIOs
> > 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.
> 
> 
> 	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Sept. 15, 2016, 8:22 a.m. UTC | #6
On Thursday, September 15, 2016 8:02:27 AM CEST Gabriele Paoloni wrote:
> 
> From <<3.1.1. Open Firmware Properties for Bus Nodes>> in 
> http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> 
> I quote:
> "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
> present.

The key here is the definition of "mapped through the bridge".
I can only understand this as "directly mapped", i.e. an I/O
port of the child bus corresponds directly to a memory address
on the parent bus, but this is not the case here.

The problem with adding the mapping here is that it looks
like it should be valid to create a page table entry for
the address returned from the translation and access it through
a pointer dereference, but that is clearly not possible.

> This is also explained quite well in
> http://lxr.free-electrons.com/source/drivers/of/address.c#L490
> 
> what do you think?

This is a separate issue, and only relevant for Apple Macintosh
machines as well as the PA-Semi sdc.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gabriele Paoloni Sept. 15, 2016, 12:05 p.m. UTC | #7
Hi Arnd

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 15 September 2016 09:22
> To: Gabriele Paoloni
> Cc: linux-arm-kernel@lists.infradead.org; Yuanzhichang;
> devicetree@vger.kernel.org; lorenzo.pieralisi@arm.com; minyard@acm.org;
> gregkh@linuxfoundation.org; benh@kernel.crashing.org; John Garry;
> will.deacon@arm.com; linux-kernel@vger.kernel.org; xuwei (O); Linuxarm;
> linux-serial@vger.kernel.org; linux-pci@vger.kernel.org;
> zourongrong@gmail.com; liviu.dudau@arm.com; kantyzc@163.com;
> zhichang.yuan02@gmail.com
> Subject: Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on
> Hip06
> 
> On Thursday, September 15, 2016 8:02:27 AM CEST Gabriele Paoloni wrote:
> >
> > From <<3.1.1. Open Firmware Properties for Bus Nodes>> in
> > http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> >
> > I quote:
> > "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
> > present.
> 
> The key here is the definition of "mapped through the bridge".
> I can only understand this as "directly mapped", i.e. an I/O
> port of the child bus corresponds directly to a memory address
> on the parent bus, but this is not the case here.
> 
> The problem with adding the mapping here is that it looks
> like it should be valid to create a page table entry for
> the address returned from the translation and access it through
> a pointer dereference, but that is clearly not possible.

I understand that somehow we are abusing of the ranges property
here however the point is that with the current implementation ranges
is needed because otherwise the ipmi driver probe will fail here:

of_ipmi_probe -> of_address_to_resource -> __of_address_to_resource
-> of_translate_address -> __of_translate_address

Now we had a bit of discussion internally and to avoid
having ranges we came up with two possible solutions:

1) Using bit 3 of phys.hi cell in 2.2.1 of
http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
This would mean reworking of_bus_isa_get_flags in 
http://lxr.free-electrons.com/source/drivers/of/address.c#L398
and setting a new flag to be checked in __of_address_to_resource

2) Adding a property in the bindings of each device that is
a child of our LPC bus and modify __of_address_to_resource
to check if the property is in the DT and eventually bypass
of_translate_address

However in both 1) and 2) there are some issues:
in 1) we are not complying with the isa binding doc (we use
a bit that should be zero); in 2) we need to modify the
bindings documentation of the devices that are connected
to our LPC controller (therefore modifying other devices
bindings to fit our special case).

I think that maybe having the 1:1 range mapping doesn't
reflect well the reality but it is the less painful
solution...

What's your view?
 
Many Thanks

Gab

> 
> > This is also explained quite well in
> > http://lxr.free-electrons.com/source/drivers/of/address.c#L490
> >
> > what do you think?
> 
> This is a separate issue, and only relevant for Apple Macintosh
> machines as well as the PA-Semi sdc.
> 
> 	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Sept. 15, 2016, 12:24 p.m. UTC | #8
On Thursday, September 15, 2016 12:05:51 PM CEST Gabriele Paoloni wrote:
> > -----Original Message-----
> > On Thursday, September 15, 2016 8:02:27 AM CEST Gabriele Paoloni wrote:
> > >
> > > From <<3.1.1. Open Firmware Properties for Bus Nodes>> in
> > > http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> > >
> > > I quote:
> > > "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
> > > present.
> > 
> > The key here is the definition of "mapped through the bridge".
> > I can only understand this as "directly mapped", i.e. an I/O
> > port of the child bus corresponds directly to a memory address
> > on the parent bus, but this is not the case here.
> > 
> > The problem with adding the mapping here is that it looks
> > like it should be valid to create a page table entry for
> > the address returned from the translation and access it through
> > a pointer dereference, but that is clearly not possible.
> 
> I understand that somehow we are abusing of the ranges property
> here however the point is that with the current implementation ranges
> is needed because otherwise the ipmi driver probe will fail here:
> 
> of_ipmi_probe -> of_address_to_resource -> __of_address_to_resource
> -> of_translate_address -> __of_translate_address
> 
> Now we had a bit of discussion internally and to avoid
> having ranges we came up with two possible solutions:
> 
> 1) Using bit 3 of phys.hi cell in 2.2.1 of
> http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> This would mean reworking of_bus_isa_get_flags in 
> http://lxr.free-electrons.com/source/drivers/of/address.c#L398
> and setting a new flag to be checked in __of_address_to_resource
> 
> 2) Adding a property in the bindings of each device that is
> a child of our LPC bus and modify __of_address_to_resource
> to check if the property is in the DT and eventually bypass
> of_translate_address
> 
> However in both 1) and 2) there are some issues:
> in 1) we are not complying with the isa binding doc (we use
> a bit that should be zero); in 2) we need to modify the
> bindings documentation of the devices that are connected
> to our LPC controller (therefore modifying other devices
> bindings to fit our special case).
> 
> I think that maybe having the 1:1 range mapping doesn't
> reflect well the reality but it is the less painful
> solution...
> 
> What's your view?

We can check the 'i' bit for I/O space in of_bus_isa_get_flags,
and that should be enough to translate the I/O port number.

The only part we need to change here is to not go through
the crazy conversion all the way from PCI I/O space to a
physical address and back to a (logical) port number
that we do today with of_translate_address/pci_address_to_pio.

I can think of a several of ways to fix __of_address_to_resource
to just do the right thing according to the ISA binding to
make the normal drivers work.

The easiest solution is probably to hook into the
"taddr == OF_BAD_ADDR" case in __of_address_to_resource
and add a lookup for ISA buses there, and instead check
if some special I/O port operations were registered
for the port number, using an architecture specific
function that arm64 implements. Other architectures
like x86 that don't have a direct mapping between I/O
ports and MMIO addresses would implement that same
function differently.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gabriele Paoloni Sept. 15, 2016, 2:28 p.m. UTC | #9
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 15 September 2016 13:25
> To: linux-arm-kernel@lists.infradead.org
> Cc: Gabriele Paoloni; devicetree@vger.kernel.org;
> lorenzo.pieralisi@arm.com; minyard@acm.org; linux-pci@vger.kernel.org;
> gregkh@linuxfoundation.org; John Garry; will.deacon@arm.com; linux-
> kernel@vger.kernel.org; Yuanzhichang; Linuxarm; xuwei (O); linux-
> serial@vger.kernel.org; benh@kernel.crashing.org;
> zourongrong@gmail.com; liviu.dudau@arm.com; kantyzc@163.com;
> zhichang.yuan02@gmail.com
> Subject: Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on
> Hip06
> 
> On Thursday, September 15, 2016 12:05:51 PM CEST Gabriele Paoloni
> wrote:
> > > -----Original Message-----
> > > On Thursday, September 15, 2016 8:02:27 AM CEST Gabriele Paoloni
> wrote:
> > > >
> > > > From <<3.1.1. Open Firmware Properties for Bus Nodes>> in
> > > > http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> > > >
> > > > I quote:
> > > > "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
> > > > present.
> > >
> > > The key here is the definition of "mapped through the bridge".
> > > I can only understand this as "directly mapped", i.e. an I/O
> > > port of the child bus corresponds directly to a memory address
> > > on the parent bus, but this is not the case here.
> > >
> > > The problem with adding the mapping here is that it looks
> > > like it should be valid to create a page table entry for
> > > the address returned from the translation and access it through
> > > a pointer dereference, but that is clearly not possible.
> >
> > I understand that somehow we are abusing of the ranges property
> > here however the point is that with the current implementation ranges
> > is needed because otherwise the ipmi driver probe will fail here:
> >
> > of_ipmi_probe -> of_address_to_resource -> __of_address_to_resource
> > -> of_translate_address -> __of_translate_address
> >
> > Now we had a bit of discussion internally and to avoid
> > having ranges we came up with two possible solutions:
> >
> > 1) Using bit 3 of phys.hi cell in 2.2.1 of
> > http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> > This would mean reworking of_bus_isa_get_flags in
> > http://lxr.free-electrons.com/source/drivers/of/address.c#L398
> > and setting a new flag to be checked in __of_address_to_resource
> >
> > 2) Adding a property in the bindings of each device that is
> > a child of our LPC bus and modify __of_address_to_resource
> > to check if the property is in the DT and eventually bypass
> > of_translate_address
> >
> > However in both 1) and 2) there are some issues:
> > in 1) we are not complying with the isa binding doc (we use
> > a bit that should be zero); in 2) we need to modify the
> > bindings documentation of the devices that are connected
> > to our LPC controller (therefore modifying other devices
> > bindings to fit our special case).
> >
> > I think that maybe having the 1:1 range mapping doesn't
> > reflect well the reality but it is the less painful
> > solution...
> >
> > What's your view?
> 
> We can check the 'i' bit for I/O space in of_bus_isa_get_flags,
> and that should be enough to translate the I/O port number.
> 
> The only part we need to change here is to not go through
> the crazy conversion all the way from PCI I/O space to a
> physical address and back to a (logical) port number
> that we do today with of_translate_address/pci_address_to_pio.
> 
> I can think of a several of ways to fix __of_address_to_resource
> to just do the right thing according to the ISA binding to
> make the normal drivers work.
> 
> The easiest solution is probably to hook into the
> "taddr == OF_BAD_ADDR" case in __of_address_to_resource
> and add a lookup for ISA buses there, and instead check
> if some special I/O port operations were registered
> for the port number, using an architecture specific
> function that arm64 implements. Other architectures
> like x86 that don't have a direct mapping between I/O
> ports and MMIO addresses would implement that same
> function differently.

So with respect to this patchset once we enter the
"taddr == OF_BAD_ADDR" case you would add an arch
specific function that checks if the resource is an I/O 
resource, if the parent node is an ISA bus (calling 
of_bus_isa_match) and if arm64_extio_ops is non NULL. 
 
I think it can work for us and it doesn't affect current
devices. I will talk to Zhichang about this for the next
patchset version.

Many Thanks for your ideas

Gab


> 
> 	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
zhichang Sept. 21, 2016, 10:09 a.m. UTC | #10
Hi, Arnd,



On 2016年09月15日 20:24, Arnd Bergmann wrote:
> On Thursday, September 15, 2016 12:05:51 PM CEST Gabriele Paoloni wrote:
>>> -----Original Message-----
>>> On Thursday, September 15, 2016 8:02:27 AM CEST Gabriele Paoloni wrote:
>>>>
>>>> From <<3.1.1. Open Firmware Properties for Bus Nodes>> in
>>>> http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
>>>>
>>>> I quote:
>>>> "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
>>>> present.
>>>
>>> The key here is the definition of "mapped through the bridge".
>>> I can only understand this as "directly mapped", i.e. an I/O
>>> port of the child bus corresponds directly to a memory address
>>> on the parent bus, but this is not the case here.
>>>
>>> The problem with adding the mapping here is that it looks
>>> like it should be valid to create a page table entry for
>>> the address returned from the translation and access it through
>>> a pointer dereference, but that is clearly not possible.
>>
>> I understand that somehow we are abusing of the ranges property
>> here however the point is that with the current implementation ranges
>> is needed because otherwise the ipmi driver probe will fail here:
>>
>> of_ipmi_probe -> of_address_to_resource -> __of_address_to_resource
>> -> of_translate_address -> __of_translate_address
>>
>> Now we had a bit of discussion internally and to avoid
>> having ranges we came up with two possible solutions:
>>
>> 1) Using bit 3 of phys.hi cell in 2.2.1 of
>> http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
>> This would mean reworking of_bus_isa_get_flags in 
>> http://lxr.free-electrons.com/source/drivers/of/address.c#L398
>> and setting a new flag to be checked in __of_address_to_resource
>>
>> 2) Adding a property in the bindings of each device that is
>> a child of our LPC bus and modify __of_address_to_resource
>> to check if the property is in the DT and eventually bypass
>> of_translate_address
>>
>> However in both 1) and 2) there are some issues:
>> in 1) we are not complying with the isa binding doc (we use
>> a bit that should be zero); in 2) we need to modify the
>> bindings documentation of the devices that are connected
>> to our LPC controller (therefore modifying other devices
>> bindings to fit our special case).
>>
>> I think that maybe having the 1:1 range mapping doesn't
>> reflect well the reality but it is the less painful
>> solution...
>>
>> What's your view?
> 
> We can check the 'i' bit for I/O space in of_bus_isa_get_flags,
> and that should be enough to translate the I/O port number.
> 
> The only part we need to change here is to not go through
> the crazy conversion all the way from PCI I/O space to a
> physical address and back to a (logical) port number
> that we do today with of_translate_address/pci_address_to_pio.
> 
Sorry for the late response! Several days' leave....
Do you want to bypass of_translate_address and pci_address_to_pio for the registered specific PIO?
I think the bypass for of_translate_address is ok, but worry some new issues will emerge without the
conversion between physical address and logical/linux port number.

When PCI host bridge which support IO operations is configured and enabled, the pci_address_to_pio will
populate the logical IO range from ZERO for the first host bridge. Our LPC will also use part of the IO range
started from ZERO. It will make in/out enter the wrong branch possibly.

In V2, the 0 - 0x1000 logical IO range is reserved for LPC use only. But it seems not so good. In this way,
PCI has no chance to use low 4K IO range(logical).

So, in V3, applying the conversion from physical/cpu address to logical/linux IO port for any IO ranges,
including the LPC, but recorded the logical IO range for LPC. When calling in/out with a logical port address,
we can check this port fall into LPC logical IO range and get back the real IO.

Do you have further comments about this??


> I can think of a several of ways to fix __of_address_to_resource
> to just do the right thing according to the ISA binding to
> make the normal drivers work.
> 
> The easiest solution is probably to hook into the
> "taddr == OF_BAD_ADDR" case in __of_address_to_resource
> and add a lookup for ISA buses there, and instead check
> if some special I/O port operations were registered
> for the port number, using an architecture specific
> function that arm64 implements. Other architectures
> like x86 that don't have a direct mapping between I/O
> ports and MMIO addresses would implement that same
> function differently.

What about add the specific quirk for Hip06 LPC in of_empty_ranges_quirk()??

you know, there are several cases in which of_translate_address return OF_BAD_ADDR.
And if we only check the special port range, it seems a bit risky. If some device want to use this port range
when no hip06 LPC is configured, the checking does not work. I think we should also check the relevant device.


Best,
Zhichang


> 
> 	Arnd
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gabriele Paoloni Sept. 21, 2016, 4:20 p.m. UTC | #11
SGkgWmhpY2hhbmcNCg0KPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiB6aGlj
aGFuZyBbbWFpbHRvOnpoaWNoYW5nLnl1YW4wMkBnbWFpbC5jb21dDQo+IFNlbnQ6IDIxIFNlcHRl
bWJlciAyMDE2IDExOjA5DQo+IFRvOiBBcm5kIEJlcmdtYW5uOyBsaW51eC1hcm0ta2VybmVsQGxp
c3RzLmluZnJhZGVhZC5vcmcNCj4gQ2M6IEdhYnJpZWxlIFBhb2xvbmk7IGRldmljZXRyZWVAdmdl
ci5rZXJuZWwub3JnOw0KPiBsb3JlbnpvLnBpZXJhbGlzaUBhcm0uY29tOyBtaW55YXJkQGFjbS5v
cmc7IGxpbnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmc7DQo+IGdyZWdraEBsaW51eGZvdW5kYXRpb24u
b3JnOyBKb2huIEdhcnJ5OyB3aWxsLmRlYWNvbkBhcm0uY29tOyBsaW51eC0NCj4ga2VybmVsQHZn
ZXIua2VybmVsLm9yZzsgWXVhbnpoaWNoYW5nOyBMaW51eGFybTsgeHV3ZWkgKE8pOyBsaW51eC0N
Cj4gc2VyaWFsQHZnZXIua2VybmVsLm9yZzsgYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOw0KPiB6
b3Vyb25ncm9uZ0BnbWFpbC5jb207IGxpdml1LmR1ZGF1QGFybS5jb207IGthbnR5emNAMTYzLmNv
bQ0KPiBTdWJqZWN0OiBSZTogW1BBVENIIFYzIDIvNF0gQVJNNjQgTFBDOiBMUEMgZHJpdmVyIGlt
cGxlbWVudGF0aW9uIG9uDQo+IEhpcDA2DQo+IA0KPiBIaSwgQXJuZCwNCj4gDQo+IA0KPiANCj4g
T24gMjAxNuW5tDA55pyIMTXml6UgMjA6MjQsIEFybmQgQmVyZ21hbm4gd3JvdGU6DQo+ID4gT24g
VGh1cnNkYXksIFNlcHRlbWJlciAxNSwgMjAxNiAxMjowNTo1MSBQTSBDRVNUIEdhYnJpZWxlIFBh
b2xvbmkNCj4gd3JvdGU6DQo+ID4+PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+Pj4g
T24gVGh1cnNkYXksIFNlcHRlbWJlciAxNSwgMjAxNiA4OjAyOjI3IEFNIENFU1QgR2FicmllbGUg
UGFvbG9uaQ0KPiB3cm90ZToNCj4gPj4+Pg0KPiA+Pj4+IEZyb20gPDwzLjEuMS4gT3BlbiBGaXJt
d2FyZSBQcm9wZXJ0aWVzIGZvciBCdXMgTm9kZXM+PiBpbg0KPiA+Pj4+IGh0dHA6Ly93d3cuZmly
bXdhcmUub3JnLzEyNzUvYmluZGluZ3MvaXNhL2lzYTBfNGQucHMNCj4gPj4+Pg0KPiA+Pj4+IEkg
cXVvdGU6DQo+ID4+Pj4gIlRoZXJlIHNoYWxsIGJlIGFuIGVudHJ5IGluIHRoZSAicmFuZ2VzIiBw
cm9wZXJ0eSBmb3IgZWFjaA0KPiA+Pj4+IG9mIHRoZSBNZW1vcnkgYW5kL29yIEkvTyBzcGFjZXMg
aWYgdGhhdCBhZGRyZXNzIHNwYWNlIGlzDQo+ID4+Pj4gbWFwcGVkIHRocm91Z2ggdGhlIGJyaWRn
ZS4iDQo+ID4+Pj4NCj4gPj4+PiBJdCBzZWVtcyB0byBtZSB0aGF0IGl0IGlzIG9rIHRvIGhhdmUg
MToxIGFkZHJlc3MgbWFwcGluZyBhbmQgdGhhdA0KPiA+Pj4+IHRoZXJlZm9yZSBvZl90cmFuc2xh
dGVfYWRkcmVzcygpIHNob3VsZCBmYWlsIGlmICJyYW5nZXMiIGlzIG5vdA0KPiA+Pj4+IHByZXNl
bnQuDQo+ID4+Pg0KPiA+Pj4gVGhlIGtleSBoZXJlIGlzIHRoZSBkZWZpbml0aW9uIG9mICJtYXBw
ZWQgdGhyb3VnaCB0aGUgYnJpZGdlIi4NCj4gPj4+IEkgY2FuIG9ubHkgdW5kZXJzdGFuZCB0aGlz
IGFzICJkaXJlY3RseSBtYXBwZWQiLCBpLmUuIGFuIEkvTw0KPiA+Pj4gcG9ydCBvZiB0aGUgY2hp
bGQgYnVzIGNvcnJlc3BvbmRzIGRpcmVjdGx5IHRvIGEgbWVtb3J5IGFkZHJlc3MNCj4gPj4+IG9u
IHRoZSBwYXJlbnQgYnVzLCBidXQgdGhpcyBpcyBub3QgdGhlIGNhc2UgaGVyZS4NCj4gPj4+DQo+
ID4+PiBUaGUgcHJvYmxlbSB3aXRoIGFkZGluZyB0aGUgbWFwcGluZyBoZXJlIGlzIHRoYXQgaXQg
bG9va3MNCj4gPj4+IGxpa2UgaXQgc2hvdWxkIGJlIHZhbGlkIHRvIGNyZWF0ZSBhIHBhZ2UgdGFi
bGUgZW50cnkgZm9yDQo+ID4+PiB0aGUgYWRkcmVzcyByZXR1cm5lZCBmcm9tIHRoZSB0cmFuc2xh
dGlvbiBhbmQgYWNjZXNzIGl0IHRocm91Z2gNCj4gPj4+IGEgcG9pbnRlciBkZXJlZmVyZW5jZSwg
YnV0IHRoYXQgaXMgY2xlYXJseSBub3QgcG9zc2libGUuDQo+ID4+DQo+ID4+IEkgdW5kZXJzdGFu
ZCB0aGF0IHNvbWVob3cgd2UgYXJlIGFidXNpbmcgb2YgdGhlIHJhbmdlcyBwcm9wZXJ0eQ0KPiA+
PiBoZXJlIGhvd2V2ZXIgdGhlIHBvaW50IGlzIHRoYXQgd2l0aCB0aGUgY3VycmVudCBpbXBsZW1l
bnRhdGlvbg0KPiByYW5nZXMNCj4gPj4gaXMgbmVlZGVkIGJlY2F1c2Ugb3RoZXJ3aXNlIHRoZSBp
cG1pIGRyaXZlciBwcm9iZSB3aWxsIGZhaWwgaGVyZToNCj4gPj4NCj4gPj4gb2ZfaXBtaV9wcm9i
ZSAtPiBvZl9hZGRyZXNzX3RvX3Jlc291cmNlIC0+IF9fb2ZfYWRkcmVzc190b19yZXNvdXJjZQ0K
PiA+PiAtPiBvZl90cmFuc2xhdGVfYWRkcmVzcyAtPiBfX29mX3RyYW5zbGF0ZV9hZGRyZXNzDQo+
ID4+DQo+ID4+IE5vdyB3ZSBoYWQgYSBiaXQgb2YgZGlzY3Vzc2lvbiBpbnRlcm5hbGx5IGFuZCB0
byBhdm9pZA0KPiA+PiBoYXZpbmcgcmFuZ2VzIHdlIGNhbWUgdXAgd2l0aCB0d28gcG9zc2libGUg
c29sdXRpb25zOg0KPiA+Pg0KPiA+PiAxKSBVc2luZyBiaXQgMyBvZiBwaHlzLmhpIGNlbGwgaW4g
Mi4yLjEgb2YNCj4gPj4gaHR0cDovL3d3dy5maXJtd2FyZS5vcmcvMTI3NS9iaW5kaW5ncy9pc2Ev
aXNhMF80ZC5wcw0KPiA+PiBUaGlzIHdvdWxkIG1lYW4gcmV3b3JraW5nIG9mX2J1c19pc2FfZ2V0
X2ZsYWdzIGluDQo+ID4+IGh0dHA6Ly9seHIuZnJlZS1lbGVjdHJvbnMuY29tL3NvdXJjZS9kcml2
ZXJzL29mL2FkZHJlc3MuYyNMMzk4DQo+ID4+IGFuZCBzZXR0aW5nIGEgbmV3IGZsYWcgdG8gYmUg
Y2hlY2tlZCBpbiBfX29mX2FkZHJlc3NfdG9fcmVzb3VyY2UNCj4gPj4NCj4gPj4gMikgQWRkaW5n
IGEgcHJvcGVydHkgaW4gdGhlIGJpbmRpbmdzIG9mIGVhY2ggZGV2aWNlIHRoYXQgaXMNCj4gPj4g
YSBjaGlsZCBvZiBvdXIgTFBDIGJ1cyBhbmQgbW9kaWZ5IF9fb2ZfYWRkcmVzc190b19yZXNvdXJj
ZQ0KPiA+PiB0byBjaGVjayBpZiB0aGUgcHJvcGVydHkgaXMgaW4gdGhlIERUIGFuZCBldmVudHVh
bGx5IGJ5cGFzcw0KPiA+PiBvZl90cmFuc2xhdGVfYWRkcmVzcw0KPiA+Pg0KPiA+PiBIb3dldmVy
IGluIGJvdGggMSkgYW5kIDIpIHRoZXJlIGFyZSBzb21lIGlzc3VlczoNCj4gPj4gaW4gMSkgd2Ug
YXJlIG5vdCBjb21wbHlpbmcgd2l0aCB0aGUgaXNhIGJpbmRpbmcgZG9jICh3ZSB1c2UNCj4gPj4g
YSBiaXQgdGhhdCBzaG91bGQgYmUgemVybyk7IGluIDIpIHdlIG5lZWQgdG8gbW9kaWZ5IHRoZQ0K
PiA+PiBiaW5kaW5ncyBkb2N1bWVudGF0aW9uIG9mIHRoZSBkZXZpY2VzIHRoYXQgYXJlIGNvbm5l
Y3RlZA0KPiA+PiB0byBvdXIgTFBDIGNvbnRyb2xsZXIgKHRoZXJlZm9yZSBtb2RpZnlpbmcgb3Ro
ZXIgZGV2aWNlcw0KPiA+PiBiaW5kaW5ncyB0byBmaXQgb3VyIHNwZWNpYWwgY2FzZSkuDQo+ID4+
DQo+ID4+IEkgdGhpbmsgdGhhdCBtYXliZSBoYXZpbmcgdGhlIDE6MSByYW5nZSBtYXBwaW5nIGRv
ZXNuJ3QNCj4gPj4gcmVmbGVjdCB3ZWxsIHRoZSByZWFsaXR5IGJ1dCBpdCBpcyB0aGUgbGVzcyBw
YWluZnVsDQo+ID4+IHNvbHV0aW9uLi4uDQo+ID4+DQo+ID4+IFdoYXQncyB5b3VyIHZpZXc/DQo+
ID4NCj4gPiBXZSBjYW4gY2hlY2sgdGhlICdpJyBiaXQgZm9yIEkvTyBzcGFjZSBpbiBvZl9idXNf
aXNhX2dldF9mbGFncywNCj4gPiBhbmQgdGhhdCBzaG91bGQgYmUgZW5vdWdoIHRvIHRyYW5zbGF0
ZSB0aGUgSS9PIHBvcnQgbnVtYmVyLg0KPiA+DQo+ID4gVGhlIG9ubHkgcGFydCB3ZSBuZWVkIHRv
IGNoYW5nZSBoZXJlIGlzIHRvIG5vdCBnbyB0aHJvdWdoDQo+ID4gdGhlIGNyYXp5IGNvbnZlcnNp
b24gYWxsIHRoZSB3YXkgZnJvbSBQQ0kgSS9PIHNwYWNlIHRvIGENCj4gPiBwaHlzaWNhbCBhZGRy
ZXNzIGFuZCBiYWNrIHRvIGEgKGxvZ2ljYWwpIHBvcnQgbnVtYmVyDQo+ID4gdGhhdCB3ZSBkbyB0
b2RheSB3aXRoIG9mX3RyYW5zbGF0ZV9hZGRyZXNzL3BjaV9hZGRyZXNzX3RvX3Bpby4NCj4gPg0K
PiBTb3JyeSBmb3IgdGhlIGxhdGUgcmVzcG9uc2UhIFNldmVyYWwgZGF5cycgbGVhdmUuLi4uDQo+
IERvIHlvdSB3YW50IHRvIGJ5cGFzcyBvZl90cmFuc2xhdGVfYWRkcmVzcyBhbmQgcGNpX2FkZHJl
c3NfdG9fcGlvIGZvcg0KPiB0aGUgcmVnaXN0ZXJlZCBzcGVjaWZpYyBQSU8/DQo+IEkgdGhpbmsg
dGhlIGJ5cGFzcyBmb3Igb2ZfdHJhbnNsYXRlX2FkZHJlc3MgaXMgb2ssIGJ1dCB3b3JyeSBzb21l
IG5ldw0KPiBpc3N1ZXMgd2lsbCBlbWVyZ2Ugd2l0aG91dCB0aGUNCj4gY29udmVyc2lvbiBiZXR3
ZWVuIHBoeXNpY2FsIGFkZHJlc3MgYW5kIGxvZ2ljYWwvbGludXggcG9ydCBudW1iZXIuDQo+IA0K
PiBXaGVuIFBDSSBob3N0IGJyaWRnZSB3aGljaCBzdXBwb3J0IElPIG9wZXJhdGlvbnMgaXMgY29u
ZmlndXJlZCBhbmQNCj4gZW5hYmxlZCwgdGhlIHBjaV9hZGRyZXNzX3RvX3BpbyB3aWxsDQo+IHBv
cHVsYXRlIHRoZSBsb2dpY2FsIElPIHJhbmdlIGZyb20gWkVSTyBmb3IgdGhlIGZpcnN0IGhvc3Qg
YnJpZGdlLiBPdXINCj4gTFBDIHdpbGwgYWxzbyB1c2UgcGFydCBvZiB0aGUgSU8gcmFuZ2UNCj4g
c3RhcnRlZCBmcm9tIFpFUk8uIEl0IHdpbGwgbWFrZSBpbi9vdXQgZW50ZXIgdGhlIHdyb25nIGJy
YW5jaCBwb3NzaWJseS4NCj4gDQo+IEluIFYyLCB0aGUgMCAtIDB4MTAwMCBsb2dpY2FsIElPIHJh
bmdlIGlzIHJlc2VydmVkIGZvciBMUEMgdXNlIG9ubHkuDQo+IEJ1dCBpdCBzZWVtcyBub3Qgc28g
Z29vZC4gSW4gdGhpcyB3YXksDQo+IFBDSSBoYXMgbm8gY2hhbmNlIHRvIHVzZSBsb3cgNEsgSU8g
cmFuZ2UobG9naWNhbCkuDQo+IA0KPiBTbywgaW4gVjMsIGFwcGx5aW5nIHRoZSBjb252ZXJzaW9u
IGZyb20gcGh5c2ljYWwvY3B1IGFkZHJlc3MgdG8NCj4gbG9naWNhbC9saW51eCBJTyBwb3J0IGZv
ciBhbnkgSU8gcmFuZ2VzLA0KPiBpbmNsdWRpbmcgdGhlIExQQywgYnV0IHJlY29yZGVkIHRoZSBs
b2dpY2FsIElPIHJhbmdlIGZvciBMUEMuIFdoZW4NCj4gY2FsbGluZyBpbi9vdXQgd2l0aCBhIGxv
Z2ljYWwgcG9ydCBhZGRyZXNzLA0KPiB3ZSBjYW4gY2hlY2sgdGhpcyBwb3J0IGZhbGwgaW50byBM
UEMgbG9naWNhbCBJTyByYW5nZSBhbmQgZ2V0IGJhY2sgdGhlDQo+IHJlYWwgSU8uDQo+IA0KPiBE
byB5b3UgaGF2ZSBmdXJ0aGVyIGNvbW1lbnRzIGFib3V0IHRoaXM/Pw0KDQpJIHRoaW5rIHRoZXJl
IGFyZSB0d28gc2VwYXJhdGUgaXNzdWVzIHRvIGJlIGRpc2N1c3NlZDoNCg0KVGhlIGZpcnN0IGlz
c3VlIGlzIGFib3V0IGhhdmluZyBvZl90cmFuc2xhdGVfYWRkcmVzcyBmYWlsaW5nIGR1ZSB0bw0K
InJhbmdlIiBtaXNzaW5nLiBBYm91dCB0aGlzIEFybmQgc3VnZ2VzdGVkIHRoYXQgaXQgaXMgbm90
IGFwcHJvcHJpYXRlDQp0byBoYXZlIGEgcmFuZ2UgZGVzY3JpYmluZyBhIGJyaWRnZSAxOjEgbWFw
cGluZyBhbmQgdGhpcyB3YXMgZGlzY3Vzc2VkDQpiZWZvcmUgaW4gdGhpcyB0aHJlYWQuIEFybmQg
aGFkIGEgc3VnZ2VzdGlvbiBhYm91dCB0aGlzIChzZWUgYmVsb3cpIA0KaG93ZXZlciAobG9va2lu
ZyB0d2ljZSBhdCB0aGUgY29kZSkgaXQgc2VlbXMgdG8gbWUgdGhhdCBzdWNoIHNvbHV0aW9uIA0K
d291bGQgbGVhZCB0byBxdWl0ZSBzb21lIGR1cGxpY2F0aW9uIGZyb20gX19vZl90cmFuc2xhdGVf
YWRkcmVzcygpDQppbiBvcmRlciB0byByZXRyaWV2ZSB0aGUgYWN0dWFsIGFkZHIgZnJvbSBkdC4u
Lg0KDQpJIHRoaW5rIGV4dGVuZGluZyBvZl9lbXB0eV9yYW5nZXNfcXVpcmsoKSBtYXkgYmUgYSBy
ZWFzb25hYmxlIHNvbHV0aW9uLg0KV2hhdCBkbyB5b3UgdGhpbmsgQXJuZD8NCiAgDQpUaGUgc2Vj
b25kIGlzc3VlIGlzIGEgY29uZmxpY3QgYmV0d2VlbiBjcHUgYWRkcmVzc2VzIHVzZWQgYnkgdGhl
IExQQw0KY29udHJvbGxlciBhbmQgaS9vIHRva2VucyBmcm9tIHBjaSBlbmRwb2ludHMuDQoNCkFi
b3V0IHRoaXMgd2hhdCBpZiB3ZSBtb2RpZnkgYXJtbjY0X2V4dGlvX29wcyB0byBoYXZlIGEgbGlz
dCBvZiByYW5nZXMNCnJhdGhlciB0aGFuIG9ubHkgb25lIHJhbmdlIChub3cgd2UgaGF2ZSBqdXN0
IHN0YXJ0L2VuZCk7IHRoZW4gaW4gdGhlDQpMUEMgZHJpdmVyIHdlIGNhbiBzY2FuIHRoZSBMUEMg
Y2hpbGQgZGV2aWNlcyBhbmQgDQoxKSBwb3B1bGF0ZSBzdWNoIGxpc3Qgb2YgcmFuZ2VzDQoyKSBj
YWxsIHBjaV9yZWdpc3Rlcl9pb19yYW5nZSBmb3Igc3VjaCByYW5nZXMNCg0KVGhlbiB3aGVuIGNh
bGxpbmcgX19vZl9hZGRyZXNzX3RvX3Jlc291cmNlIHdlIHJldHJpZXZlIEkvTyB0b2tlbnMgDQpm
b3IgdGhlIGRldmljZXMgb24gdG9wIG9mIHRoZSBMUEMgZHJpdmVyIGFuZCBpbiB0aGUgSS9PIGFj
Y2Vzc29ycw0Kd2UgY2FsbCBwY2lfcGlvX3RvX2FkZHJlc3MgdG8gZmlndXJlIG91dCB0aGUgY3B1
IGFkZHJlc3MgYW5kIGNvbXBhcmUNCml0IHRvIHRoZSBsaXN0IG9mIHJhbmdlcyBpbiBhcm1uNjRf
ZXh0aW9fb3BzLg0KICANCldoYXQgYWJvdXQgdGhpcz8NCg0KVGhhbmtzDQoNCkdhYg0KDQo+IA0K
PiANCj4gPiBJIGNhbiB0aGluayBvZiBhIHNldmVyYWwgb2Ygd2F5cyB0byBmaXggX19vZl9hZGRy
ZXNzX3RvX3Jlc291cmNlDQo+ID4gdG8ganVzdCBkbyB0aGUgcmlnaHQgdGhpbmcgYWNjb3JkaW5n
IHRvIHRoZSBJU0EgYmluZGluZyB0bw0KPiA+IG1ha2UgdGhlIG5vcm1hbCBkcml2ZXJzIHdvcmsu
DQo+ID4NCj4gPiBUaGUgZWFzaWVzdCBzb2x1dGlvbiBpcyBwcm9iYWJseSB0byBob29rIGludG8g
dGhlDQo+ID4gInRhZGRyID09IE9GX0JBRF9BRERSIiBjYXNlIGluIF9fb2ZfYWRkcmVzc190b19y
ZXNvdXJjZQ0KPiA+IGFuZCBhZGQgYSBsb29rdXAgZm9yIElTQSBidXNlcyB0aGVyZSwgYW5kIGlu
c3RlYWQgY2hlY2sNCj4gPiBpZiBzb21lIHNwZWNpYWwgSS9PIHBvcnQgb3BlcmF0aW9ucyB3ZXJl
IHJlZ2lzdGVyZWQNCj4gPiBmb3IgdGhlIHBvcnQgbnVtYmVyLCB1c2luZyBhbiBhcmNoaXRlY3R1
cmUgc3BlY2lmaWMNCj4gPiBmdW5jdGlvbiB0aGF0IGFybTY0IGltcGxlbWVudHMuIE90aGVyIGFy
Y2hpdGVjdHVyZXMNCj4gPiBsaWtlIHg4NiB0aGF0IGRvbid0IGhhdmUgYSBkaXJlY3QgbWFwcGlu
ZyBiZXR3ZWVuIEkvTw0KPiA+IHBvcnRzIGFuZCBNTUlPIGFkZHJlc3NlcyB3b3VsZCBpbXBsZW1l
bnQgdGhhdCBzYW1lDQo+ID4gZnVuY3Rpb24gZGlmZmVyZW50bHkuDQo+IA0KPiBXaGF0IGFib3V0
IGFkZCB0aGUgc3BlY2lmaWMgcXVpcmsgZm9yIEhpcDA2IExQQyBpbg0KPiBvZl9lbXB0eV9yYW5n
ZXNfcXVpcmsoKT8/DQo+IA0KPiB5b3Uga25vdywgdGhlcmUgYXJlIHNldmVyYWwgY2FzZXMgaW4g
d2hpY2ggb2ZfdHJhbnNsYXRlX2FkZHJlc3MgcmV0dXJuDQo+IE9GX0JBRF9BRERSLg0KPiBBbmQg
aWYgd2Ugb25seSBjaGVjayB0aGUgc3BlY2lhbCBwb3J0IHJhbmdlLCBpdCBzZWVtcyBhIGJpdCBy
aXNreS4gSWYNCj4gc29tZSBkZXZpY2Ugd2FudCB0byB1c2UgdGhpcyBwb3J0IHJhbmdlDQo+IHdo
ZW4gbm8gaGlwMDYgTFBDIGlzIGNvbmZpZ3VyZWQsIHRoZSBjaGVja2luZyBkb2VzIG5vdCB3b3Jr
LiBJIHRoaW5rIHdlDQo+IHNob3VsZCBhbHNvIGNoZWNrIHRoZSByZWxldmFudCBkZXZpY2UuDQo+
IA0KPiANCj4gQmVzdCwNCj4gWmhpY2hhbmcNCj4gDQo+IA0KPiA+DQo+ID4gCUFybmQNCj4gPg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Sept. 21, 2016, 8:18 p.m. UTC | #12
On Wednesday, September 21, 2016 4:20:55 PM CEST Gabriele Paoloni wrote:
> > -----Original Message-----
> > From: zhichang [mailto:zhichang.yuan02@gmail.com]
> > On 2016年09月15日 20:24, Arnd Bergmann wrote:
> > > On Thursday, September 15, 2016 12:05:51 PM CEST Gabriele Paoloni
> > wrote:
> > >>> -----Original Message-----
> > >>> On Thursday, September 15, 2016 8:02:27 AM CEST Gabriele Paoloni
> > wrote:
> > >> I think that maybe having the 1:1 range mapping doesn't
> > >> reflect well the reality but it is the less painful
> > >> solution...
> > >>
> > >> What's your view?
> > >
> > > We can check the 'i' bit for I/O space in of_bus_isa_get_flags,
> > > and that should be enough to translate the I/O port number.
> > >
> > > The only part we need to change here is to not go through
> > > the crazy conversion all the way from PCI I/O space to a
> > > physical address and back to a (logical) port number
> > > that we do today with of_translate_address/pci_address_to_pio.
> > >
> > Sorry for the late response! Several days' leave....
> > Do you want to bypass of_translate_address and pci_address_to_pio for
> > the registered specific PIO?
> > I think the bypass for of_translate_address is ok, but worry some new
> > issues will emerge without the
> > conversion between physical address and logical/linux port number.

The same function that handles the non-translated region would
do that conversion.

> > When PCI host bridge which support IO operations is configured and
> > enabled, the pci_address_to_pio will
> > populate the logical IO range from ZERO for the first host bridge. Our
> > LPC will also use part of the IO range
> > started from ZERO. It will make in/out enter the wrong branch possibly.
> > 
> > In V2, the 0 - 0x1000 logical IO range is reserved for LPC use only.
> > But it seems not so good. In this way,
> > PCI has no chance to use low 4K IO range(logical).
> > 
> > So, in V3, applying the conversion from physical/cpu address to
> > logical/linux IO port for any IO ranges,
> > including the LPC, but recorded the logical IO range for LPC. When
> > calling in/out with a logical port address,
> > we can check this port fall into LPC logical IO range and get back the
> > real IO.

Right, and the same translation can be used in __of_address_to_resource()
going the opposite way.

> > Do you have further comments about this??
> 
> I think there are two separate issues to be discussed:
> 
> The first issue is about having of_translate_address failing due to
> "range" missing. About this Arnd suggested that it is not appropriate
> to have a range describing a bridge 1:1 mapping and this was discussed
> before in this thread. Arnd had a suggestion about this (see below) 
> however (looking twice at the code) it seems to me that such solution 
> would lead to quite some duplication from __of_translate_address()
> in order to retrieve the actual addr from dt...

I don't think we need to duplicate much, we can probably safely
assume that there are no nontrivial ranges in devices below the LPC
node, so we just walk up the bus to see if the node is a child
(or possibly grandchild etc) of the LPC bus, and treat any IO port
number under there as a physical port number, which has a known
offset from the Linux I/O port number.

> I think extending of_empty_ranges_quirk() may be a reasonable solution.
> What do you think Arnd?

I don't really like that idea, that quirk is meant to work around
broken DTs, but we can just make the DT valid and implement the
code properly.

> The second issue is a conflict between cpu addresses used by the LPC
> controller and i/o tokens from pci endpoints.
> 
> About this what if we modify armn64_extio_ops to have a list of ranges
> rather than only one range (now we have just start/end); then in the
> LPC driver we can scan the LPC child devices and 
> 1) populate such list of ranges
> 2) call pci_register_io_range for such ranges

Scanning the child devices sounds really wrong, please register just
one range that covers the bus to keep the workaround as simple
as possible.

> Then when calling __of_address_to_resource we retrieve I/O tokens 
> for the devices on top of the LPC driver and in the I/O accessors
> we call pci_pio_to_address to figure out the cpu address and compare
> it to the list of ranges in armn64_extio_ops.
>   
> What about this?

That seems really complex for something that can be quite simple.
The only thing we need to worry about is that the io_range_list
contains an entry for the LPC bus so we don't conflict with the
PCI buses.

	Arnd

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Masters Oct. 2, 2016, 10:03 p.m. UTC | #13
On 09/14/2016 02:32 PM, Arnd Bergmann wrote:
> On Wednesday, September 14, 2016 10:50:44 PM CEST zhichang.yuan wrote:

>> 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.

Just for the record, Arnd has the right idea. There is only one type of
UART permitted by SBSA (PL011). We carved out an exception for a design
that was already in flight and allowed it to be 16550. That other design
was then corrected in future generations to be PL011 as we required it
to be. Then there's the Hip06. I've given feedback elsewhere about the
need for there to be (at most) two types of UART in the wild. This "LPC"
stuff needs cleaning up (feedback given elsewhere already on that), but
we won't be adding a third serial driver into the mix in order to make
it work. There will be standard ARM servers. There will not be the
kinda-sorta-standard. Thanks.

Jon.

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John Garry Oct. 4, 2016, 12:02 p.m. UTC | #14
On 02/10/2016 23:03, Jon Masters wrote:
> On 09/14/2016 02:32 PM, Arnd Bergmann wrote:
>> On Wednesday, September 14, 2016 10:50:44 PM CEST zhichang.yuan wrote:
>
>>> 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.
>
> Just for the record, Arnd has the right idea. There is only one type of
> UART permitted by SBSA (PL011). We carved out an exception for a design
> that was already in flight and allowed it to be 16550. That other design
> was then corrected in future generations to be PL011 as we required it
> to be. Then there's the Hip06. I've given feedback elsewhere about the
> need for there to be (at most) two types of UART in the wild. This "LPC"
> stuff needs cleaning up (feedback given elsewhere already on that), but
> we won't be adding a third serial driver into the mix in order to make
> it work. There will be standard ARM servers. There will not be the
> kinda-sorta-standard. Thanks.
>

Right, so I think Zhichang can make the necessary generic changes to 
8250 OF driver to support IO port as well as MMIO-based.

However an LPC-based earlycon driver is still required.

A note on hip07-based D05 (for those unaware): this does not use 
LPC-based uart. It uses PL011. The hardware guys have managed some 
trickery where they loopback the serial line around the BMC/CPLD. But we 
still need it for hip06 D03 and any other boards which want to use LPC 
bus for uart.

A question on SBSA: does it propose how to provide serial via BMC for SOL?


> Jon.
>
>
> .
>


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Herrenschmidt Oct. 6, 2016, 12:18 a.m. UTC | #15
On Tue, 2016-10-04 at 13:02 +0100, John Garry wrote:
> Right, so I think Zhichang can make the necessary generic changes to 
> 8250 OF driver to support IO port as well as MMIO-based.
> 
> However an LPC-based earlycon driver is still required.
> 
> A note on hip07-based D05 (for those unaware): this does not use 
> LPC-based uart. It uses PL011. The hardware guys have managed some 
> trickery where they loopback the serial line around the BMC/CPLD. But we 
> still need it for hip06 D03 and any other boards which want to use LPC 
> bus for uart.
> 
> A question on SBSA: does it propose how to provide serial via BMC for SOL?

Probably another reason to keep 8250 as a legal option ... The (very
popular) Aspeed BMCs tend to do this via a 8250-looking virtual UART on
LPC.

Cheers,
Ben,

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John Garry Oct. 6, 2016, 1:31 p.m. UTC | #16
On 06/10/2016 01:18, Benjamin Herrenschmidt wrote:
> On Tue, 2016-10-04 at 13:02 +0100, John Garry wrote:
>> Right, so I think Zhichang can make the necessary generic changes to
>> 8250 OF driver to support IO port as well as MMIO-based.
>>
>> However an LPC-based earlycon driver is still required.
>>
>> A note on hip07-based D05 (for those unaware): this does not use
>> LPC-based uart. It uses PL011. The hardware guys have managed some
>> trickery where they loopback the serial line around the BMC/CPLD. But we
>> still need it for hip06 D03 and any other boards which want to use LPC
>> bus for uart.
>>
>> A question on SBSA: does it propose how to provide serial via BMC for SOL?
>
> Probably another reason to keep 8250 as a legal option ... The (very
> popular) Aspeed BMCs tend to do this via a 8250-looking virtual UART on
> LPC.
>
> Cheers,
> Ben,

I think we're talking about the same thing for our LPC-based UART.

John

>
>
> .
>


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
new file mode 100644
index 0000000..820e26d
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
@@ -0,0 +1,35 @@ 
+Hisilicon Hip06 low-pin-count device
+  Usually LPC controller is part of PCI host bridge, so the legacy ISA ports
+  locate on LPC bus can be accessed direclty. But some SoCs have independent
+  LPC controller, and access the legacy ports by triggering LPC I/O cycles.
+  Hisilicon Hip06 implements this LPC device.
+
+Required properties:
+- compatible: should be "hisilicon,low-pin-count"
+- #address-cells: must be 2 which stick to the ISA/EISA binding doc.
+- #size-cells: must be 1 which stick to the ISA/EISA binding doc.
+- reg: base address and length of the register set for the device.
+- ranges: define a 1:1 mapping between the I/O space of the child device and
+	  the parent.
+
+Note:
+  The node name before '@' must be "isa" to represent the binding stick to the
+  ISA/EISA binding specification.
+
+Example:
+
+isa@a01b0000 {
+	compatible = "hisilicom,low-pin-count";
+	#address-cells = <2>;
+	#size-cells = <1>;
+	reg = <0x0 0xa01b0000 0x0 0x1000>;
+	ranges = <0x01 0xe4 0x0 0xe4 0x1000>;
+
+
+	ipmi0: bt@e4 {
+		compatible = "ipmi-bt";
+		device_type = "ipmi";
+		reg = <0x01 0xe4 0x04>;
+		status = "disabled";
+	};
+};
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 3b205e2..fdb232b 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -64,6 +64,14 @@  config BRCMSTB_GISB_ARB
 	  arbiter. This driver provides timeout and target abort error handling
 	  and internal bus master decoding.
 
+config HISILICON_LPC
+	bool "Workaround for nonstandard ISA I/O space on Hisilicon Hip0X"
+	depends on (ARCH_HISI || COMPILE_TEST) && ARM64
+	select ARM64_INDIRECT_PIO
+	help
+	  Driver needed for some legacy ISA devices attached to Low-Pin-Count
+	  on Hisilicon Hip0X Soc.
+
 config IMX_WEIM
 	bool "Freescale EIM DRIVER"
 	depends on ARCH_MXC
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index ac84cc4..6ffbb27 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -7,6 +7,8 @@  obj-$(CONFIG_ARM_CCI)		+= arm-cci.o
 obj-$(CONFIG_ARM_CCN)		+= arm-ccn.o
 
 obj-$(CONFIG_BRCMSTB_GISB_ARB)	+= brcmstb_gisb.o
+obj-$(CONFIG_ARM64_INDIRECT_PIO)	+= extio.o
+obj-$(CONFIG_HISILICON_LPC)	+= hisi_lpc.o
 obj-$(CONFIG_IMX_WEIM)		+= imx-weim.o
 obj-$(CONFIG_MIPS_CDMM)		+= mips_cdmm.o
 obj-$(CONFIG_MVEBU_MBUS) 	+= mvebu-mbus.o
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
new file mode 100644
index 0000000..9b364d0
--- /dev/null
+++ b/drivers/bus/hisi_lpc.c
@@ -0,0 +1,653 @@ 
+/*
+ * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved.
+ * Author: Zhichang Yuan <yuanzhichang@hisilicon.com>
+ * Author: Zou Rongrong <@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/acpi.h>
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/pci.h>
+#include <linux/serial_8250.h>
+#include <linux/slab.h>
+
+
+struct hisilpc_dev;
+
+/* This flag is specific to differentiate earlycon operations and the others */
+#define FG_EARLYCON_LPC		0x0001
+/*
+ * this bit set means each IO operation will target to different port address;
+ * 0 means repeatly IO operations will be sticked on the same port, such as BT;
+ */
+#define FG_INCRADDR_LPC		0x0002
+
+struct lpc_cycle_para {
+	unsigned int opflags;
+	unsigned int csize;/*the data length of each operation*/
+};
+
+struct hisilpc_dev {
+	spinlock_t cycle_lock;
+	void __iomem  *membase;
+	struct platform_device *pltdev;
+};
+
+
+/* The maximum continous operations*/
+#define LPC_MAX_OPCNT	16
+
+#define LPC_REG_START		(0x00)/*start a new LPC cycle*/
+#define LPC_REG_OP_STATUS	(0x04)/*the current LPC status*/
+#define LPC_REG_IRQ_ST		(0x08)/*interrupt enable&status*/
+#define LPC_REG_OP_LEN		(0x10)/*how many LPC cycles each start*/
+#define LPC_REG_CMD		(0x14)/*command for the required LPC cycle*/
+#define LPC_REG_ADDR		(0x20)/*LPC target address*/
+#define LPC_REG_WDATA		(0x24)/*data to be written*/
+#define LPC_REG_RDATA		(0x28)/*data coming from peer*/
+
+
+/* The command register fields*/
+#define LPC_CMD_SAMEADDR_SING	(0x00000008)
+#define LPC_CMD_SAMEADDR_INC	(0x00000000)
+#define LPC_CMD_TYPE_IO		(0x00000000)
+#define LPC_CMD_TYPE_MEM	(0x00000002)
+#define LPC_CMD_TYPE_FWH	(0x00000004)
+#define LPC_CMD_WRITE		(0x00000001)
+#define LPC_CMD_READ		(0x00000000)
+
+#define LPC_IRQ_CLEAR		(0x02)
+#define LPC_IRQ_OCCURRED	(0x02)
+
+#define LPC_STATUS_IDLE		(0x01)
+#define LPC_OP_FINISHED		(0x02)
+
+#define START_WORK		(0x01)
+
+/*
+ * The minimal waiting interval... Suggest it is not less than 10.
+ * Bigger value probably will lower the performance.
+ */
+#define LPC_NSEC_PERWAIT	100
+/*
+ * The maximum waiting time is about 128us.
+ * The fastest IO cycle time is about 390ns, but the worst case will wait
+ * for extra 256 lpc clocks, so (256 + 13) * 30ns = 8 us. The maximum
+ * burst cycles is 16. So, the maximum waiting time is about 128us under
+ * worst case.
+ * choose 1300 as the maximum.
+ */
+#define LPC_MAX_WAITCNT		1300
+/* About 10us. This is specfic for single IO operation, such as inb. */
+#define LPC_PEROP_WAITCNT	100
+
+
+static inline int wait_lpc_idle(unsigned char *mbase,
+				unsigned int waitcnt) {
+	u32 opstatus = 0;
+
+	while (waitcnt--) {
+		ndelay(LPC_NSEC_PERWAIT);
+		opstatus = readl(mbase + LPC_REG_OP_STATUS);
+		if (opstatus & LPC_STATUS_IDLE)
+			return (opstatus & LPC_OP_FINISHED) ? 0 : (-EIO);
+	}
+	return -ETIME;
+}
+
+
+/**
+ * hisilpc_target_in - trigger a series of lpc cycles to read required data
+ *		  from target periperal.
+ * @pdev: pointer to hisi lpc device
+ * @para: some paramerters used to control the lpc I/O operations
+ * @ptaddr: the lpc I/O target port address
+ * @buf: where the read back data is stored
+ * @opcnt: how many I/O operations required in this calling
+ *
+ * only one byte data is read each I/O operation.
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static int hisilpc_target_in(struct hisilpc_dev *pdev,
+				struct lpc_cycle_para *para,
+				unsigned long ptaddr, unsigned char *buf,
+				unsigned long opcnt)
+{
+	unsigned int cmd_word;
+	unsigned int waitcnt;
+	int retval;
+	/*initialized as 0 to remove compile warning */
+	unsigned long flags = 0;
+
+	if (!buf || !opcnt || !para || !pdev)
+		return -EINVAL;
+
+	if (para->csize != 1 || opcnt  > LPC_MAX_OPCNT)
+		return -EINVAL;
+
+	cmd_word = LPC_CMD_TYPE_IO | LPC_CMD_READ;
+	waitcnt = (LPC_PEROP_WAITCNT);
+	if (!(para->opflags & FG_INCRADDR_LPC)) {
+		cmd_word |= LPC_CMD_SAMEADDR_SING;
+		waitcnt = LPC_MAX_WAITCNT;
+	}
+
+	/* whole operation must be atomic */
+	if (!(para->opflags & FG_EARLYCON_LPC))
+		spin_lock_irqsave(&pdev->cycle_lock, flags);
+
+	writel(opcnt, pdev->membase + LPC_REG_OP_LEN);
+
+	writel(cmd_word, pdev->membase + LPC_REG_CMD);
+
+	writel(ptaddr, pdev->membase + LPC_REG_ADDR);
+
+	writel(START_WORK, pdev->membase + LPC_REG_START);
+
+	/* whether the operation is finished */
+	retval = wait_lpc_idle(pdev->membase, waitcnt);
+	if (!retval) {
+		for (; opcnt--; buf++)
+			*buf = readl(pdev->membase + LPC_REG_RDATA);
+	}
+
+	if (!(para->opflags & FG_EARLYCON_LPC))
+		spin_unlock_irqrestore(&pdev->cycle_lock, flags);
+
+	return retval;
+}
+
+/**
+ * hisilpc_target_out - trigger a series of lpc cycles to write required data
+ *		  to target periperal.
+ * @pdev: pointer to hisi lpc device
+ * @para: some paramerters used to control the lpc I/O operations
+ * @ptaddr: the lpc I/O target port address
+ * @buf: where the data to be written is stored
+ * @opcnt: how many I/O operations required
+ *
+ * only one byte data is read each I/O operation.
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static int hisilpc_target_out(struct hisilpc_dev *pdev,
+				struct lpc_cycle_para *para,
+				unsigned long ptaddr,
+				const unsigned char *buf,
+				unsigned long opcnt)
+{
+	unsigned int cmd_word;
+	unsigned int waitcnt;
+	int retval;
+	/* initialized as 0 to remove compile warning */
+	unsigned long flags = 0;
+
+
+	if (!buf || !opcnt || !para || !pdev)
+		return -EINVAL;
+
+	if (para->csize != 1 || opcnt  > LPC_MAX_OPCNT)
+		return -EINVAL;
+
+	cmd_word = LPC_CMD_TYPE_IO | LPC_CMD_WRITE;
+	waitcnt = (LPC_PEROP_WAITCNT);
+	if (!(para->opflags & FG_INCRADDR_LPC)) {
+		cmd_word |= LPC_CMD_SAMEADDR_SING;
+		waitcnt = LPC_MAX_WAITCNT;
+	}
+
+	/* whole operation must be atomic */
+	if (!(para->opflags & FG_EARLYCON_LPC))
+		spin_lock_irqsave(&pdev->cycle_lock, flags);
+
+	writel(opcnt, pdev->membase + LPC_REG_OP_LEN);
+	for (; opcnt--; buf++)
+		writel(*buf, pdev->membase + LPC_REG_WDATA);
+
+	writel(cmd_word, pdev->membase + LPC_REG_CMD);
+
+	writel(ptaddr, pdev->membase + LPC_REG_ADDR);
+
+	writel(START_WORK, pdev->membase + LPC_REG_START);
+
+	/* whether the operation is finished */
+	retval = wait_lpc_idle(pdev->membase, waitcnt);
+
+	if (!(para->opflags & FG_EARLYCON_LPC))
+		spin_unlock_irqrestore(&pdev->cycle_lock, flags);
+
+	return retval;
+}
+
+/**
+ * hisilpc_comm_inb - read/input the data from the I/O peripheral through LPC.
+ * @devobj: pointer to the device information relevant to LPC controller.
+ * @outbuf: a buffer where the data read is stored at.
+ * @ptaddr: the target I/O port address.
+ * @dlen: the data length required to read from the target I/O port.
+ * @count: how many I/O operations required in this calling.  >1 is for ins.
+ *
+ * For this lpc, only support inb/insb now.
+ *
+ * For inbs, returns 0 on success, -1 on fail.
+ * when succeed, the data read back is stored in buffer pointed by inbuf.
+ * For inb, return the data read from I/O or -1 when error occur.
+ */
+u64 hisilpc_comm_inb(void *devobj, unsigned long ptaddr,
+				void *inbuf, size_t dlen,
+				unsigned int count)
+{
+	struct hisilpc_dev *lpcdev;
+	struct lpc_cycle_para iopara;
+	unsigned int loopcnt, cntleft;
+	unsigned int rd_data;
+	unsigned char *newbuf;
+	int ret = 0;
+	/* only support data unit length is 1 now... */
+	if (!count || (!inbuf && count != 1) || !devobj || dlen != 1)
+		return -1;
+
+	newbuf = (unsigned char *)inbuf;
+	/*
+	 * the operation data len is 4 bytes, need to ensure the buffer
+	 * is big enough.
+	 */
+	if (!inbuf || count < sizeof(u32))
+		newbuf = (unsigned char *)&rd_data;
+
+	lpcdev = (struct hisilpc_dev *)devobj;
+	dev_dbg(&lpcdev->pltdev->dev, "In-IO(0x%lx), count=%u\n", ptaddr,
+			count);
+
+	iopara.opflags = FG_INCRADDR_LPC;
+	/*
+	 * to improve performance, support repeatly rd at same target
+	 * address.
+	 */
+	if (count > 1)
+		iopara.opflags &= ~FG_INCRADDR_LPC;
+
+	iopara.csize = dlen;
+
+	cntleft = count;
+	do {
+		loopcnt = (cntleft > LPC_MAX_OPCNT) ? LPC_MAX_OPCNT : cntleft;
+		ret = hisilpc_target_in(lpcdev,
+				&iopara, ptaddr, newbuf, loopcnt);
+		if (ret)
+			return -1;
+		newbuf += loopcnt;
+		cntleft -= loopcnt;
+	} while (cntleft);
+
+	/* for inb */
+	if (!inbuf)
+		return rd_data;
+	/* for insb, copy the data to the return variable */
+	if (inbuf != newbuf)
+		memcpy(inbuf, &rd_data, count);
+
+	return 0;
+}
+
+/**
+ * hisilpc_comm_outb - write/output the data in out buffer to the I/O peripheral
+ *		    through LPC.
+ * @devobj: pointer to the device information relevant to LPC controller.
+ * @outbuf: a buffer where the data to be written is stored.
+ * @ptaddr: the target I/O port address.
+ * @dlen: the data length required writing to the target I/O port .
+ * @count: how many I/O operations required in this calling. >1 is for outs.
+ *
+ * For this lpc, only support outb/outsb now.
+ *
+ */
+void hisilpc_comm_outb(void *devobj, unsigned long ptaddr,
+				const void *outbuf, size_t dlen,
+				unsigned int count)
+{
+	struct hisilpc_dev *lpcdev;
+	struct lpc_cycle_para iopara;
+	unsigned int loopcnt;
+	const unsigned char *newbuf;
+	int ret = 0;
+
+	if (!count || !outbuf || !devobj)
+		return;
+
+	newbuf = (const unsigned char *)outbuf;
+	lpcdev = (struct hisilpc_dev *)devobj;
+
+	dev_dbg(&lpcdev->pltdev->dev, "Out-IO(0x%lx), cnt=%u\n", ptaddr, count);
+
+	iopara.opflags = FG_INCRADDR_LPC;
+	/* to improve performance,  support repeatly wr same target address */
+	if (count > 1)
+		iopara.opflags &= ~FG_INCRADDR_LPC;
+
+	iopara.csize = 1;
+
+	do {
+		loopcnt = (count > LPC_MAX_OPCNT) ? LPC_MAX_OPCNT : count;
+		ret = hisilpc_target_out(lpcdev,
+				&iopara, ptaddr, newbuf, loopcnt);
+		if (ret)
+			return;
+		newbuf += loopcnt;
+		count -= loopcnt;
+	} while (count);
+}
+
+
+/**
+ * hisilpc_ischild_ipmi - check whether the designated device is ipmi
+ * @dev: the device to be checked.
+ * @data: the value used to match the acpi device in checking.
+ *
+ * Returns 1 means the device to be checked is matched.
+ * 0 means some failures.
+ *
+ */
+static int  hisilpc_ischild_ipmi(struct device *dev)
+{
+	struct acpi_device *adev;
+	struct acpi_hardware_id *hwid;
+	/* only support dts and acpi */
+	if (IS_ERR_OR_NULL(dev->fwnode) && !dev->of_node) {
+		dev_err(dev, "Not valid child device!\n");
+		return -EINVAL;
+	}
+
+	adev = ACPI_COMPANION(dev);
+	if (adev) {
+		list_for_each_entry(hwid, &adev->pnp.ids, list) {
+			dev_info(dev, "hwid is %s\n", hwid->id);
+			if (!strcmp("IPI0001", hwid->id))
+				return 1;
+		}
+	} else {
+		if (!strcmp(dev->of_node->type, "ipmi"))
+			return 1;
+	}
+
+	dev_info(dev, "not ipmi child device!\n");
+	return 0;
+}
+
+
+/**
+ * hisilpc_children_map_sysio - setup the mapping between system Io and
+ *			physical IO
+ *
+ * @child: the device whose IO is handling
+ * @data: some device specific data. For ACPI device, should be NULL.
+ *
+ * Returns >=0 means the mapping is successfully created;
+ * others mean some failures.
+ */
+static int hisilpc_children_map_sysio(struct device * child, void * data)
+{
+	struct resource *iores;
+	unsigned long cpuio;
+	struct extio_ops *opsnode;
+	int ret;
+	struct hisilpc_dev *lpcdev;
+
+	if (!child || !child->parent)
+		return -EINVAL;
+
+	iores = platform_get_resource_byname(to_platform_device(child),
+					IORESOURCE_IO, "dev_io");
+	if (!iores)
+		return -ENODEV;
+
+	/*
+	 * can not use devm_kzalloc to allocate slab for child before its driver
+	 * start probing. Here allocate the slab with the name of parent.
+	 */
+	opsnode = devm_kzalloc(child->parent, sizeof(*opsnode), GFP_KERNEL);
+	if (!opsnode)
+		return -ENOMEM;
+
+	cpuio = data ? *((unsigned long *)data) : 0;
+
+	opsnode->start = iores->start;
+	opsnode->end = iores->end;
+	opsnode->ptoffset = cpuio ? (cpuio - iores->start) : 0;
+
+	dev_info(child, "map sys port[%lx - %lx] offset=0x%lx",
+				(unsigned long)iores->start,
+				(unsigned long)iores->end,
+				opsnode->ptoffset);
+
+	opsnode->pfin = hisilpc_comm_inb;
+	opsnode->pfout = hisilpc_comm_outb;
+
+	lpcdev = platform_get_drvdata(to_platform_device(child->parent));
+	opsnode->devpara = lpcdev;
+
+	/* only apply indirect-IO to ipmi child device */
+	ret = hisilpc_ischild_ipmi(child);
+	if (ret < 0)
+		return ret;
+
+	if (ret > 0) {
+		WRITE_ONCE(arm64_extio_ops, opsnode);
+		/* update ret as successful */
+		ret = 0;
+	} else {
+		ret = platform_device_add_data(to_platform_device(child),
+						opsnode,
+						sizeof(*opsnode));
+		devm_kfree(child->parent, opsnode);
+	}
+
+	if (!ret)
+		dev_info(child, "to phy [%lx-%lx]\n", cpuio,
+			(unsigned long)(iores->end - iores->start) + cpuio);
+	else
+		dev_info(child, "FAIL(0x%x)!!\n", ret);
+
+	return ret;
+}
+
+/**
+ * of_hisilpc_register_pio - register the deivce physical IO address into
+ *			io_range_list
+ *
+ * Returns >=0 means ok.
+ * others mean some failures.
+ */
+static int of_hisilpc_register_pio(struct device_node *dev,
+					unsigned long *phyport)
+{
+	static int isfirst = 0;
+	const __be32	*addrp;
+	u64		size;
+	unsigned int	flags;
+	u64	taddr;
+	int residx = 0;
+
+
+	if (!dev || !phyport)
+		return -EINVAL;
+	/* only one IO reg exists */
+	do {
+		addrp = of_get_address(dev, residx, &size, &flags);
+		if (addrp == NULL) {
+			pr_err("%s:: get OF address(%d) FAIL!\n",
+				dev->name, residx);
+			return -EINVAL;
+		}
+		residx++;
+	} while(!(flags & IORESOURCE_IO));
+
+
+	taddr = of_translate_address(dev, addrp);
+	if (taddr == OF_BAD_ADDR) {
+		pr_err("%s:: translate IO address fail\n", dev->name);
+		return -EINVAL;
+	}
+
+	/* register one more IO byte for the first register */
+	if (!isfirst) {
+		size += 1;
+		isfirst = 1;
+	}
+
+	if (pci_register_io_range(taddr, size)) {
+		pr_err("%s::register physical range[%llx, %llx) FAIL!\n",
+			dev->name, taddr, size);
+		return -ENXIO;
+	}
+
+	pr_info("%s:: register physical range[%llx, %llx) OK\n",
+			dev->name, taddr, size);
+
+	*phyport = taddr;
+
+	return 0;
+}
+
+/**
+ * hisilpc_probe_child_dev - setup the mapping between linux IO and
+ *			physical IO for all children under hisilpc
+ *
+ * @ppdev: point to the hisilpc device
+ *
+ * Returns =0 means ok.
+ * others mean some failures.
+ */
+static int hisilpc_probe_child_dev(struct device *ppdev)
+{
+	int ret;
+
+	if (!ppdev)
+		return -EINVAL;
+
+	ret = 0;
+	/* for device tree, scan the child devices now */
+	if (!has_acpi_companion(ppdev)) {
+		struct device_node *root, *child;
+
+		root = ppdev->of_node;
+		for_each_available_child_of_node(root, child) {
+			struct platform_device *ptdev;
+			unsigned long cpuio;
+
+			/* register the IO range configured in dt */
+			ret = of_hisilpc_register_pio(child, &cpuio);
+			if (ret) {
+				dev_err(ppdev, "fail to register raw IO for %s\n",
+						child->name);
+				return ret;
+			}
+
+			ptdev = of_platform_device_create(child, NULL, ppdev);
+			if (!ptdev) {
+				dev_err(ppdev, "create platform device fail for %s\n",
+					child->name);
+				return -EFAULT;
+			}
+
+			ret = hisilpc_children_map_sysio(&ptdev->dev, &cpuio);
+			if (ret)
+				dev_err(&ptdev->dev, "Mapping sysio for dts child devices FAIL\n");
+		}
+	} else {
+		ret = device_for_each_child(ppdev, NULL,
+					hisilpc_children_map_sysio);
+		if (ret)
+			dev_err(ppdev, "Mapping sysio for ACPI child devices FAIL\n");
+	}
+
+	return ret;
+}
+
+
+/**
+ * hisilpc_probe - the probe callback function for hisi lpc device,
+ *		will finish all the intialization.
+ * @pdev: the platform device corresponding to hisi lpc
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static int hisilpc_probe(struct platform_device *pdev)
+{
+	struct resource *iores;
+	struct hisilpc_dev *lpcdev;
+	int ret;
+
+	dev_info(&pdev->dev, "hslpc start probing...\n");
+
+	lpcdev = devm_kzalloc(&pdev->dev,
+				sizeof(struct hisilpc_dev), GFP_KERNEL);
+	if (!lpcdev)
+		return -ENOMEM;
+
+	spin_lock_init(&lpcdev->cycle_lock);
+
+	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	lpcdev->membase = devm_ioremap_resource(&pdev->dev, iores);
+	if (IS_ERR(lpcdev->membase)) {
+		dev_err(&pdev->dev, "No mem resource\n");
+		return PTR_ERR(lpcdev->membase);
+	}
+
+	lpcdev->pltdev = pdev;
+	platform_set_drvdata(pdev, lpcdev);
+
+	ret = hisilpc_probe_child_dev(&pdev->dev);
+	if (!ret)
+		dev_info(&pdev->dev, "hslpc finish probing...\n");
+	else
+		dev_err(&pdev->dev, "hslpc probe got fail(%d)!\n", -ret);
+
+	return ret;
+}
+
+
+static const struct of_device_id hisilpc_of_match[] = {
+	{
+		.compatible = "hisilicon,low-pin-count",
+	},
+	{},
+};
+
+static const struct acpi_device_id hisilpc_acpi_match[] = {
+	{"HISI0191", },
+	{},
+};
+
+static struct platform_driver hisilpc_driver = {
+	.driver = {
+		.name           = "hisi_lpc",
+		.of_match_table = hisilpc_of_match,
+		.acpi_match_table = hisilpc_acpi_match,
+	},
+	.probe = hisilpc_probe,
+};
+
+
+builtin_platform_driver(hisilpc_driver);
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 02b2903..ba4a330 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -690,9 +690,18 @@  static int __of_address_to_resource(struct device_node *dev,
 	memset(r, 0, sizeof(struct resource));
 	if (flags & IORESOURCE_IO) {
 		unsigned long port;
+
 		port = pci_address_to_pio(taddr);
 		if (port == (unsigned long)-1)
 			return -EINVAL;
+		/*
+		 * special processing for non-pci device gurantee the linux start pio
+		 * is not ZERO. Otherwise, some drivers' initialization will fail.
+		 */
+		if (!port && (!IS_ENABLED(CONFIG_OF_ADDRESS_PCI) ||
+				!of_bus_pci_match(dev)))
+			port += 1;
+
 		r->start = port;
 		r->end = port + size - 1;
 	} else {