Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

在 2016/1/6 21:36, Rongrong Zou 写道:

在 2016/1/5 20:19, Arnd Bergmann 写道:

On Tuesday 05 January 2016 19:59:49 Rongrong Zou wrote:

在 2016/1/5 0:34, Arnd Bergmann 写道:

On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote:

在 2016/1/4 19:13, Arnd Bergmann 写道:

On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:

在 2015/12/31 23:00, Rongrong Zou 写道:

Ranges property can set empty, but this means 1:1 translation. the I/O
port range is translated to MMIO address 0x0001  to
0x0001 0004, it looks wrong else. I wonder if anyone get legacy
I/O port resource from dts.


As I said, nothing should really require the ranges property here, unless
you have a valid IORESOURCE_MEM translation. The code that requires
the ranges to be present is wrong.



I think the openfirmware(DT) do not support for those unmapped I/O ports, 
because I
must get resource by calling of_address_to_resource(), which have to call
pci_address_to_pio() when resource type is IORESOURCE_IO. I'm sorry I have no
better idea for this now. Maybe liviu can give me some opinions.


I think on x86 it works (or used to work, few people use open firmware on
x86 these days, and it may be broken), and the pci_address_to_pio() call
behaves differently when PCI_IOBASE is set. x86 never maps I/O ports into
memory mapped I/O addresses, they have their own way of accessing them
just like your platform.



The big problem is:

The return value of_translate_address can be only an cpu addr,
there is no map from cpuaddr to I/O port in x86 as you said,
we still can't get io resource.

static int __of_address_to_resource(struct device_node *dev,
const __be32 *addrp, u64 size, unsigned int flags,
const char *name, struct resource *r)
{
...
taddr = of_translate_address(dev, addrp);
...
}



/**
   * of_address_to_resource - Translate device tree address and return as 
resource
   *
   * Note that if your address is a PIO address, the conversion will fail if
   * the physical address can't be internally converted to an IO token with
   * pci_address_to_pio(), that is because it's either called to early or it
   * can't be matched to any host bridge IO space
   */
int of_address_to_resource(struct device_node *dev, int index,
   struct resource *r)


The problem here seems to be that the code assumes that either the I/O ports
are always mapped or they are never mapped (no PCI_IOBASE). We need to extend
it because now we can have the combination of the two.



I am considering the following solution:

Adding unmapped isa io functions in

drivers/of/address.c,

static LIST_HEAD(legacy_io_range_list);
int isa_register_io_range(phys_addr_t addr, resource_size_t size);

/* before I call isa(LPC) bus driver, the input I/O port must be translated to 
phys_addr_t
(the least 16bit means port addr on bus, the second 16bit means bus id)*/

phys_addr_t isa_pio_to_bus_addr(unsigned long pio);

/* the returned PIO do not conflict with PIO get from pci_address_to_pio*/
unsigned long isa_bus_addr_to_pio(phys_addr_t address);

drivers/bus/lpc.c

lpc_bus_probe()
{
 isa_register_io_range(phys_addr_t addr, resource_size_t size);
}

inb(unsigned long port)
{
 unsigned short bus;
 phys_addr_t addr;
 /*hit isa port range*/
 if(addr = isa_pio_to_bus_addr(port))
 {
 bus = (addr >> 16) & 0x;

 call lpc driver with addr;
 return lpc_read_byte(bus, addr);
 }
 else /*not hit*/
 {
 return readb(PCI_IOBASE + port);
 }
}






For ipmi driver, I can get I/O port resource by DMI rather than dts.


No, the ipmi driver uses the resource that belongs to the platform
device already, you can't rely on DMI data to be present there.


Ipmi has a lot of way to be discovered(ACPI, DMI, hardcoded, hot-add,
openfirmware and a few other), I think we just use one of them, not all of them.
It depend on vendor's hardware solution actually.


I don't think we should mix multiple methods here: if the bus is described
in DT, all its children should be there as well. Otherwise you get into problems
e.g. if you have multiple instances of the LPC bus and the Linux I/O addresses
for one or more of them have an offset to the bus specific addresses.

The bus probe code decides what the Linux I/O port numbers are, but DMI
and other methods have no idea of the mapping. As long as there is only
one instance, using the first 0x1000 addresses with a 1:1 mapping saves
us a bit of trouble, but I'd be worried about relying on that assumption
too much.

Arnd


.


--
Thanks,

Rongrong
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

在 2016/1/5 20:19, Arnd Bergmann 写道:

On Tuesday 05 January 2016 19:59:49 Rongrong Zou wrote:

在 2016/1/5 0:34, Arnd Bergmann 写道:

On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote:

在 2016/1/4 19:13, Arnd Bergmann 写道:

On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:

在 2015/12/31 23:00, Rongrong Zou 写道:

Ranges property can set empty, but this means 1:1 translation. the I/O
port range is translated to MMIO address 0x0001  to
0x0001 0004, it looks wrong else. I wonder if anyone get legacy
I/O port resource from dts.


As I said, nothing should really require the ranges property here, unless
you have a valid IORESOURCE_MEM translation. The code that requires
the ranges to be present is wrong.



I think the openfirmware(DT) do not support for those unmapped I/O ports, 
because I
must get resource by calling of_address_to_resource(), which have to call
pci_address_to_pio() when resource type is IORESOURCE_IO. I'm sorry I have no
better idea for this now. Maybe liviu can give me some opinions.


I think on x86 it works (or used to work, few people use open firmware on
x86 these days, and it may be broken), and the pci_address_to_pio() call
behaves differently when PCI_IOBASE is set. x86 never maps I/O ports into
memory mapped I/O addresses, they have their own way of accessing them
just like your platform.


/**
   * of_address_to_resource - Translate device tree address and return as 
resource
   *
   * Note that if your address is a PIO address, the conversion will fail if
   * the physical address can't be internally converted to an IO token with
   * pci_address_to_pio(), that is because it's either called to early or it
   * can't be matched to any host bridge IO space
   */
int of_address_to_resource(struct device_node *dev, int index,
   struct resource *r)


The problem here seems to be that the code assumes that either the I/O ports
are always mapped or they are never mapped (no PCI_IOBASE). We need to extend
it because now we can have the combination of the two.



I am considering the following solution:

Adding unmapped isa io functions in

drivers/of/address.c,

static LIST_HEAD(legacy_io_range_list);
int isa_register_io_range(phys_addr_t addr, resource_size_t size);

/* before I call isa(LPC) bus driver, the input I/O port must be translated to 
phys_addr_t
(the least 16bit means port addr on bus, the second 16bit means bus id)*/

phys_addr_t isa_pio_to_bus_addr(unsigned long pio);

/* the returned PIO do not conflict with PIO get from pci_address_to_pio*/
unsigned long isa_bus_addr_to_pio(phys_addr_t address);

drivers/bus/lpc.c

lpc_bus_probe()
{
isa_register_io_range(phys_addr_t addr, resource_size_t size);
}

inb(unsigned long port)
{
unsigned short bus;
phys_addr_t addr;
/*hit isa port range*/
if(addr = isa_pio_to_bus_addr(port))
{
bus = (addr >> 16) & 0x;

call lpc driver with addr;
return lpc_read_byte(bus, addr);
}
else /*not hit*/
{
return readb(PCI_IOBASE + port);
}
}






For ipmi driver, I can get I/O port resource by DMI rather than dts.


No, the ipmi driver uses the resource that belongs to the platform
device already, you can't rely on DMI data to be present there.


Ipmi has a lot of way to be discovered(ACPI, DMI, hardcoded, hot-add,
openfirmware and a few other), I think we just use one of them, not all of them.
It depend on vendor's hardware solution actually.


I don't think we should mix multiple methods here: if the bus is described
in DT, all its children should be there as well. Otherwise you get into problems
e.g. if you have multiple instances of the LPC bus and the Linux I/O addresses
for one or more of them have an offset to the bus specific addresses.

The bus probe code decides what the Linux I/O port numbers are, but DMI
and other methods have no idea of the mapping. As long as there is only
one instance, using the first 0x1000 addresses with a 1:1 mapping saves
us a bit of trouble, but I'd be worried about relying on that assumption
too much.

Arnd


.



Thanks,

Rongrong

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

在 2016/1/5 20:19, Arnd Bergmann 写道:

On Tuesday 05 January 2016 19:59:49 Rongrong Zou wrote:

在 2016/1/5 0:34, Arnd Bergmann 写道:

On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote:

在 2016/1/4 19:13, Arnd Bergmann 写道:

On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:

在 2015/12/31 23:00, Rongrong Zou 写道:

Ranges property can set empty, but this means 1:1 translation. the I/O
port range is translated to MMIO address 0x0001  to
0x0001 0004, it looks wrong else. I wonder if anyone get legacy
I/O port resource from dts.


As I said, nothing should really require the ranges property here, unless
you have a valid IORESOURCE_MEM translation. The code that requires
the ranges to be present is wrong.



I think the openfirmware(DT) do not support for those unmapped I/O ports, 
because I
must get resource by calling of_address_to_resource(), which have to call
pci_address_to_pio() when resource type is IORESOURCE_IO. I'm sorry I have no
better idea for this now. Maybe liviu can give me some opinions.


I think on x86 it works (or used to work, few people use open firmware on
x86 these days, and it may be broken), and the pci_address_to_pio() call
behaves differently when PCI_IOBASE is set. x86 never maps I/O ports into
memory mapped I/O addresses, they have their own way of accessing them
just like your platform.


/**
   * of_address_to_resource - Translate device tree address and return as 
resource
   *
   * Note that if your address is a PIO address, the conversion will fail if
   * the physical address can't be internally converted to an IO token with
   * pci_address_to_pio(), that is because it's either called to early or it
   * can't be matched to any host bridge IO space
   */
int of_address_to_resource(struct device_node *dev, int index,
   struct resource *r)


The problem here seems to be that the code assumes that either the I/O ports
are always mapped or they are never mapped (no PCI_IOBASE). We need to extend
it because now we can have the combination of the two.



I am considering the following solution:

Adding unmapped isa io functions in

drivers/of/address.c,

static LIST_HEAD(legacy_io_range_list);
int isa_register_io_range(phys_addr_t addr, resource_size_t size);

/* before I call isa(LPC) bus driver, the input I/O port must be translated to 
phys_addr_t
(the least 16bit means port addr on bus, the second 16bit means bus id)*/

phys_addr_t isa_pio_to_bus_addr(unsigned long pio);

/* the returned PIO do not conflict with PIO get from pci_address_to_pio*/
unsigned long isa_bus_addr_to_pio(phys_addr_t address);

drivers/bus/lpc.c

lpc_bus_probe()
{
isa_register_io_range(phys_addr_t addr, resource_size_t size);
}

inb(unsigned long port)
{
unsigned short bus;
phys_addr_t addr;
/*hit isa port range*/
if(addr = isa_pio_to_bus_addr(port))
{
bus = (addr >> 16) & 0x;

call lpc driver with addr;
return lpc_read_byte(bus, addr);
}
else /*not hit*/
{
return readb(PCI_IOBASE + port);
}
}






For ipmi driver, I can get I/O port resource by DMI rather than dts.


No, the ipmi driver uses the resource that belongs to the platform
device already, you can't rely on DMI data to be present there.


Ipmi has a lot of way to be discovered(ACPI, DMI, hardcoded, hot-add,
openfirmware and a few other), I think we just use one of them, not all of them.
It depend on vendor's hardware solution actually.


I don't think we should mix multiple methods here: if the bus is described
in DT, all its children should be there as well. Otherwise you get into problems
e.g. if you have multiple instances of the LPC bus and the Linux I/O addresses
for one or more of them have an offset to the bus specific addresses.

The bus probe code decides what the Linux I/O port numbers are, but DMI
and other methods have no idea of the mapping. As long as there is only
one instance, using the first 0x1000 addresses with a 1:1 mapping saves
us a bit of trouble, but I'd be worried about relying on that assumption
too much.

Arnd


.



Thanks,

Rongrong

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

在 2016/1/6 21:36, Rongrong Zou 写道:

在 2016/1/5 20:19, Arnd Bergmann 写道:

On Tuesday 05 January 2016 19:59:49 Rongrong Zou wrote:

在 2016/1/5 0:34, Arnd Bergmann 写道:

On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote:

在 2016/1/4 19:13, Arnd Bergmann 写道:

On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:

在 2015/12/31 23:00, Rongrong Zou 写道:

Ranges property can set empty, but this means 1:1 translation. the I/O
port range is translated to MMIO address 0x0001  to
0x0001 0004, it looks wrong else. I wonder if anyone get legacy
I/O port resource from dts.


As I said, nothing should really require the ranges property here, unless
you have a valid IORESOURCE_MEM translation. The code that requires
the ranges to be present is wrong.



I think the openfirmware(DT) do not support for those unmapped I/O ports, 
because I
must get resource by calling of_address_to_resource(), which have to call
pci_address_to_pio() when resource type is IORESOURCE_IO. I'm sorry I have no
better idea for this now. Maybe liviu can give me some opinions.


I think on x86 it works (or used to work, few people use open firmware on
x86 these days, and it may be broken), and the pci_address_to_pio() call
behaves differently when PCI_IOBASE is set. x86 never maps I/O ports into
memory mapped I/O addresses, they have their own way of accessing them
just like your platform.



The big problem is:

The return value of_translate_address can be only an cpu addr,
there is no map from cpuaddr to I/O port in x86 as you said,
we still can't get io resource.

static int __of_address_to_resource(struct device_node *dev,
const __be32 *addrp, u64 size, unsigned int flags,
const char *name, struct resource *r)
{
...
taddr = of_translate_address(dev, addrp);
...
}



/**
   * of_address_to_resource - Translate device tree address and return as 
resource
   *
   * Note that if your address is a PIO address, the conversion will fail if
   * the physical address can't be internally converted to an IO token with
   * pci_address_to_pio(), that is because it's either called to early or it
   * can't be matched to any host bridge IO space
   */
int of_address_to_resource(struct device_node *dev, int index,
   struct resource *r)


The problem here seems to be that the code assumes that either the I/O ports
are always mapped or they are never mapped (no PCI_IOBASE). We need to extend
it because now we can have the combination of the two.



I am considering the following solution:

Adding unmapped isa io functions in

drivers/of/address.c,

static LIST_HEAD(legacy_io_range_list);
int isa_register_io_range(phys_addr_t addr, resource_size_t size);

/* before I call isa(LPC) bus driver, the input I/O port must be translated to 
phys_addr_t
(the least 16bit means port addr on bus, the second 16bit means bus id)*/

phys_addr_t isa_pio_to_bus_addr(unsigned long pio);

/* the returned PIO do not conflict with PIO get from pci_address_to_pio*/
unsigned long isa_bus_addr_to_pio(phys_addr_t address);

drivers/bus/lpc.c

lpc_bus_probe()
{
 isa_register_io_range(phys_addr_t addr, resource_size_t size);
}

inb(unsigned long port)
{
 unsigned short bus;
 phys_addr_t addr;
 /*hit isa port range*/
 if(addr = isa_pio_to_bus_addr(port))
 {
 bus = (addr >> 16) & 0x;

 call lpc driver with addr;
 return lpc_read_byte(bus, addr);
 }
 else /*not hit*/
 {
 return readb(PCI_IOBASE + port);
 }
}






For ipmi driver, I can get I/O port resource by DMI rather than dts.


No, the ipmi driver uses the resource that belongs to the platform
device already, you can't rely on DMI data to be present there.


Ipmi has a lot of way to be discovered(ACPI, DMI, hardcoded, hot-add,
openfirmware and a few other), I think we just use one of them, not all of them.
It depend on vendor's hardware solution actually.


I don't think we should mix multiple methods here: if the bus is described
in DT, all its children should be there as well. Otherwise you get into problems
e.g. if you have multiple instances of the LPC bus and the Linux I/O addresses
for one or more of them have an offset to the bus specific addresses.

The bus probe code decides what the Linux I/O port numbers are, but DMI
and other methods have no idea of the mapping. As long as there is only
one instance, using the first 0x1000 addresses with a 1:1 mapping saves
us a bit of trouble, but I'd be worried about relying on that assumption
too much.

Arnd


.


--
Thanks,

Rongrong
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Arnd Bergmann]

On Tuesday 05 January 2016 19:59:49 Rongrong Zou wrote:
> 在 2016/1/5 0:34, Arnd Bergmann 写道:
> > On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote:
> >> 在 2016/1/4 19:13, Arnd Bergmann 写道:
> >>> On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:
>  在 2015/12/31 23:00, Rongrong Zou 写道:
> >> Ranges property can set empty, but this means 1:1 translation. the I/O
> >> port range is translated to MMIO address 0x0001  to
> >> 0x0001 0004, it looks wrong else. I wonder if anyone get legacy
> >> I/O port resource from dts.
> >
> > As I said, nothing should really require the ranges property here, unless
> > you have a valid IORESOURCE_MEM translation. The code that requires
> > the ranges to be present is wrong.
> >
> 
> I think the openfirmware(DT) do not support for those unmapped I/O ports, 
> because I
> must get resource by calling of_address_to_resource(), which have to call
> pci_address_to_pio() when resource type is IORESOURCE_IO. I'm sorry I have no
> better idea for this now. Maybe liviu can give me some opinions.

I think on x86 it works (or used to work, few people use open firmware on
x86 these days, and it may be broken), and the pci_address_to_pio() call
behaves differently when PCI_IOBASE is set. x86 never maps I/O ports into
memory mapped I/O addresses, they have their own way of accessing them
just like your platform.

> /**
>   * of_address_to_resource - Translate device tree address and return as 
> resource
>   *
>   * Note that if your address is a PIO address, the conversion will fail if
>   * the physical address can't be internally converted to an IO token with
>   * pci_address_to_pio(), that is because it's either called to early or it
>   * can't be matched to any host bridge IO space
>   */
> int of_address_to_resource(struct device_node *dev, int index,
>  struct resource *r)

The problem here seems to be that the code assumes that either the I/O ports
are always mapped or they are never mapped (no PCI_IOBASE). We need to extend
it because now we can have the combination of the two.

> >> For ipmi driver, I can get I/O port resource by DMI rather than dts.
> >
> > No, the ipmi driver uses the resource that belongs to the platform
> > device already, you can't rely on DMI data to be present there.
> 
> Ipmi has a lot of way to be discovered(ACPI, DMI, hardcoded, hot-add,
> openfirmware and a few other), I think we just use one of them, not all of 
> them.
> It depend on vendor's hardware solution actually.

I don't think we should mix multiple methods here: if the bus is described
in DT, all its children should be there as well. Otherwise you get into problems
e.g. if you have multiple instances of the LPC bus and the Linux I/O addresses
for one or more of them have an offset to the bus specific addresses.

The bus probe code decides what the Linux I/O port numbers are, but DMI
and other methods have no idea of the mapping. As long as there is only
one instance, using the first 0x1000 addresses with a 1:1 mapping saves
us a bit of trouble, but I'd be worried about relying on that assumption
too much.

Arnd

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

在 2016/1/5 0:34, Arnd Bergmann 写道:

On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote:

在 2016/1/4 19:13, Arnd Bergmann 写道:

On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:

在 2015/12/31 23:00, Rongrong Zou 写道:
*/
  compatible = "low-pin-count";
  device_type = "isa";
  #address-cells = <2>;
  #size-cells = <1>;
  reg = <0x0 0xa01b 0x0 0x1>;
  ranges = <0x1 0x0 0x0 0x0 0x1000>;
/*
*  ranges is required, then i can get the IORESOURCE_IO <0xe4,4> from "reg = <0x1, 
0x00e4, 4>".
*
*/
  ipmi_0:ipmi@00e4{
  device_type = "ipmi";
  compatible = "ipmi-bt";
  reg = <0x1 0x00e4 0x4>;
};



This looks wrong: the property above says that the I/O port range is
translated to MMIO address 0x to 0x0001, which is not
true on your hardware. I think this needs to be changed in the code
so the ranges property is not required for I/O ports.


Ranges property can set empty, but this means 1:1 translation. the I/O
port range is translated to MMIO address 0x0001  to
0x0001 0004, it looks wrong else. I wonder if anyone get legacy
I/O port resource from dts.


As I said, nothing should really require the ranges property here, unless
you have a valid IORESOURCE_MEM translation. The code that requires
the ranges to be present is wrong.



I think the openfirmware(DT) do not support for those unmapped I/O ports, 
because I
must get resource by calling of_address_to_resource(), which have to call
pci_address_to_pio() when resource type is IORESOURCE_IO. I'm sorry I have no
better idea for this now. Maybe liviu can give me some opinions.

/**
 * of_address_to_resource - Translate device tree address and return as resource
 *
 * Note that if your address is a PIO address, the conversion will fail if
 * the physical address can't be internally converted to an IO token with
 * pci_address_to_pio(), that is because it's either called to early or it
 * can't be matched to any host bridge IO space
 */
int of_address_to_resource(struct device_node *dev, int index,
   struct resource *r)


For ipmi driver, I can get I/O port resource by DMI rather than dts.


No, the ipmi driver uses the resource that belongs to the platform
device already, you can't rely on DMI data to be present there.


Ipmi has a lot of way to be discovered(ACPI, DMI, hardcoded, hot-add,
openfirmware and a few other), I think we just use one of them, not all of them.
It depend on vendor's hardware solution actually.




Arnd

.



Thanks,

Rongrong

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

在 2016/1/5 0:34, Arnd Bergmann 写道:

On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote:

在 2016/1/4 19:13, Arnd Bergmann 写道:

On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:

在 2015/12/31 23:00, Rongrong Zou 写道:
*/
  compatible = "low-pin-count";
  device_type = "isa";
  #address-cells = <2>;
  #size-cells = <1>;
  reg = <0x0 0xa01b 0x0 0x1>;
  ranges = <0x1 0x0 0x0 0x0 0x1000>;
/*
*  ranges is required, then i can get the IORESOURCE_IO <0xe4,4> from "reg = <0x1, 
0x00e4, 4>".
*
*/
  ipmi_0:ipmi@00e4{
  device_type = "ipmi";
  compatible = "ipmi-bt";
  reg = <0x1 0x00e4 0x4>;
};



This looks wrong: the property above says that the I/O port range is
translated to MMIO address 0x to 0x0001, which is not
true on your hardware. I think this needs to be changed in the code
so the ranges property is not required for I/O ports.


Ranges property can set empty, but this means 1:1 translation. the I/O
port range is translated to MMIO address 0x0001  to
0x0001 0004, it looks wrong else. I wonder if anyone get legacy
I/O port resource from dts.


As I said, nothing should really require the ranges property here, unless
you have a valid IORESOURCE_MEM translation. The code that requires
the ranges to be present is wrong.



I think the openfirmware(DT) do not support for those unmapped I/O ports, 
because I
must get resource by calling of_address_to_resource(), which have to call
pci_address_to_pio() when resource type is IORESOURCE_IO. I'm sorry I have no
better idea for this now. Maybe liviu can give me some opinions.

/**
 * of_address_to_resource - Translate device tree address and return as resource
 *
 * Note that if your address is a PIO address, the conversion will fail if
 * the physical address can't be internally converted to an IO token with
 * pci_address_to_pio(), that is because it's either called to early or it
 * can't be matched to any host bridge IO space
 */
int of_address_to_resource(struct device_node *dev, int index,
   struct resource *r)


For ipmi driver, I can get I/O port resource by DMI rather than dts.


No, the ipmi driver uses the resource that belongs to the platform
device already, you can't rely on DMI data to be present there.


Ipmi has a lot of way to be discovered(ACPI, DMI, hardcoded, hot-add,
openfirmware and a few other), I think we just use one of them, not all of them.
It depend on vendor's hardware solution actually.




Arnd

.



Thanks,

Rongrong

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Arnd Bergmann]

On Tuesday 05 January 2016 19:59:49 Rongrong Zou wrote:
> 在 2016/1/5 0:34, Arnd Bergmann 写道:
> > On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote:
> >> 在 2016/1/4 19:13, Arnd Bergmann 写道:
> >>> On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:
>  在 2015/12/31 23:00, Rongrong Zou 写道:
> >> Ranges property can set empty, but this means 1:1 translation. the I/O
> >> port range is translated to MMIO address 0x0001  to
> >> 0x0001 0004, it looks wrong else. I wonder if anyone get legacy
> >> I/O port resource from dts.
> >
> > As I said, nothing should really require the ranges property here, unless
> > you have a valid IORESOURCE_MEM translation. The code that requires
> > the ranges to be present is wrong.
> >
> 
> I think the openfirmware(DT) do not support for those unmapped I/O ports, 
> because I
> must get resource by calling of_address_to_resource(), which have to call
> pci_address_to_pio() when resource type is IORESOURCE_IO. I'm sorry I have no
> better idea for this now. Maybe liviu can give me some opinions.

I think on x86 it works (or used to work, few people use open firmware on
x86 these days, and it may be broken), and the pci_address_to_pio() call
behaves differently when PCI_IOBASE is set. x86 never maps I/O ports into
memory mapped I/O addresses, they have their own way of accessing them
just like your platform.

> /**
>   * of_address_to_resource - Translate device tree address and return as 
> resource
>   *
>   * Note that if your address is a PIO address, the conversion will fail if
>   * the physical address can't be internally converted to an IO token with
>   * pci_address_to_pio(), that is because it's either called to early or it
>   * can't be matched to any host bridge IO space
>   */
> int of_address_to_resource(struct device_node *dev, int index,
>  struct resource *r)

The problem here seems to be that the code assumes that either the I/O ports
are always mapped or they are never mapped (no PCI_IOBASE). We need to extend
it because now we can have the combination of the two.

> >> For ipmi driver, I can get I/O port resource by DMI rather than dts.
> >
> > No, the ipmi driver uses the resource that belongs to the platform
> > device already, you can't rely on DMI data to be present there.
> 
> Ipmi has a lot of way to be discovered(ACPI, DMI, hardcoded, hot-add,
> openfirmware and a few other), I think we just use one of them, not all of 
> them.
> It depend on vendor's hardware solution actually.

I don't think we should mix multiple methods here: if the bus is described
in DT, all its children should be there as well. Otherwise you get into problems
e.g. if you have multiple instances of the LPC bus and the Linux I/O addresses
for one or more of them have an offset to the bus specific addresses.

The bus probe code decides what the Linux I/O port numbers are, but DMI
and other methods have no idea of the mapping. As long as there is only
one instance, using the first 0x1000 addresses with a 1:1 mapping saves
us a bit of trouble, but I'd be worried about relying on that assumption
too much.

Arnd

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Arnd Bergmann]

On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote:
> 在 2016/1/4 19:13, Arnd Bergmann 写道:
> > On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:
> >> 在 2015/12/31 23:00, Rongrong Zou 写道:
> >> */
> >>  compatible = "low-pin-count";
> >>  device_type = "isa";
> >>  #address-cells = <2>;
> >>  #size-cells = <1>;
> >>  reg = <0x0 0xa01b 0x0 0x1>;
> >>  ranges = <0x1 0x0 0x0 0x0 0x1000>;
> >> /*
> >> *  ranges is required, then i can get the IORESOURCE_IO <0xe4,4> from "reg 
> >> = <0x1, 0x00e4, 4>".
> >> *
> >> */
> >>  ipmi_0:ipmi@00e4{
> >>  device_type = "ipmi";
> >>  compatible = "ipmi-bt";
> >>  reg = <0x1 0x00e4 0x4>;
> >> };
> >>
> >
> > This looks wrong: the property above says that the I/O port range is
> > translated to MMIO address 0x to 0x0001, which is not
> > true on your hardware. I think this needs to be changed in the code
> > so the ranges property is not required for I/O ports.
> 
> Ranges property can set empty, but this means 1:1 translation. the I/O 
> port range is translated to MMIO address 0x0001  to 
> 0x0001 0004, it looks wrong else. I wonder if anyone get legacy 
> I/O port resource from dts.

As I said, nothing should really require the ranges property here, unless
you have a valid IORESOURCE_MEM translation. The code that requires
the ranges to be present is wrong.

> For ipmi driver, I can get I/O port resource by DMI rather than dts.

No, the ipmi driver uses the resource that belongs to the platform
device already, you can't rely on DMI data to be present there.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

在 2016/1/4 19:13, Arnd Bergmann 写道:

On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:

在 2015/12/31 23:00, Rongrong Zou 写道:

2015-12-31 22:40 GMT+08:00 Arnd Bergmann mailto:a...@arndb.de>>:
  > On Thursday 31 December 2015 22:12:19 Rongrong Zou wrote:
  > > 在 2015/12/30 17:06, Arnd Bergmann 写道:
  > > > On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
  >
  > The DT sample above looks good in principle. I believe what you are missing
  > here is code in your driver to scan the child nodes to create the platform
  > devices. of_bus_isa_translate() should work with your definition here
  > and create the correct IORESOURCE_IO resources. You don't have any MMIO
  > resources, so the absence of a ranges property is ok. Maybe all you
  > are missing is a call to of_platform_populate() or of_platform_bus_probe()?
  >

You are right. thanks, i'll try on test board .  if i get the correct result , 
the new patch
will be sent later. By the way, it's my another email account use when i at 
home.


I tried, and there need some additional changes.

isa@a01b {

/*the node name should start with "isa", because of below definition
* static int of_bus_isa_match(struct device_node *np)
* {
*   return !strcmp(np->name, "isa");
* }


Looks good. It would be nicer to match on device_type than on name,
but this is ancient code and it's probably best not to touch it
so we don't accidentally break some old SPARC or PPC system.


*/
compatible = "low-pin-count";
device_type = "isa";
#address-cells = <2>;
#size-cells = <1>;
reg = <0x0 0xa01b 0x0 0x1>;
ranges = <0x1 0x0 0x0 0x0 0x1000>;
/*
*  ranges is required, then i can get the IORESOURCE_IO <0xe4,4> from "reg = <0x1, 
0x00e4, 4>".
*
*/
ipmi_0:ipmi@00e4{
device_type = "ipmi";
compatible = "ipmi-bt";
reg = <0x1 0x00e4 0x4>;
};



This looks wrong: the property above says that the I/O port range is
translated to MMIO address 0x to 0x0001, which is not
true on your hardware. I think this needs to be changed in the code
so the ranges property is not required for I/O ports.


Ranges property can set empty, but this means 1:1 translation. the I/O 
port range is translated to MMIO address 0x0001  to 
0x0001 0004, it looks wrong else. I wonder if anyone get legacy 
I/O port resource from dts.


For ipmi driver, I can get I/O port resource by DMI rather than dts.




drivers\of\address.c
static int __of_address_to_resource(struct device_node *dev,
  const __be32 *addrp, u64 size, unsigned int flags,
  const char *name, struct resource *r)
{
  u64 taddr;

  if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
  return -EINVAL;
  taddr = of_translate_address(dev, addrp);
  if (taddr == OF_BAD_ADDR)
  return -EINVAL;
  memset(r, 0, sizeof(struct resource));
  if (flags & IORESOURCE_IO) {
  unsigned long port;

/*/
/*legacy port(< 0x1000) is reserved, and need no translation here*/
/*/
  if(taddr + size < PCIBIOS_MIN_IO){
  r->start = taddr;
  r->end = taddr + size - 1;
  }


I don't like having a special case based on the address here,
the same kind of hack might be needed for PCI I/O spaces in
hardware that uses an indirect method like your LPC bus
does, and the code above will not work on any LPC implementation
that correctly multiplexes its I/O ports with the first PCI domain.

I think it would be better to avoid translating the port into
a physical address to start with just to translate it back into
a port number, what we need instead is the offset between the
bus specific port number and the linux port number. I've added
Liviu to Cc, he wrote this code originally and may have some idea
of how we could do that.

Arnd


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Arnd Bergmann]

On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:
> 在 2015/12/31 23:00, Rongrong Zou 写道:
> > 2015-12-31 22:40 GMT+08:00 Arnd Bergmann  > >:
> >  > On Thursday 31 December 2015 22:12:19 Rongrong Zou wrote:
> >  > > 在 2015/12/30 17:06, Arnd Bergmann 写道:
> >  > > > On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
> >  >
> >  > The DT sample above looks good in principle. I believe what you are 
> > missing
> >  > here is code in your driver to scan the child nodes to create the 
> > platform
> >  > devices. of_bus_isa_translate() should work with your definition here
> >  > and create the correct IORESOURCE_IO resources. You don't have any MMIO
> >  > resources, so the absence of a ranges property is ok. Maybe all you
> >  > are missing is a call to of_platform_populate() or 
> > of_platform_bus_probe()?
> >  >
> >
> > You are right. thanks, i'll try on test board .  if i get the correct 
> > result , the new patch
> > will be sent later. By the way, it's my another email account use when i at 
> > home.
> 
> I tried, and there need some additional changes.
> 
> isa@a01b {
> 
> /*the node name should start with "isa", because of below definition
> * static int of_bus_isa_match(struct device_node *np)
> * {
> * return !strcmp(np->name, "isa");
> * }

Looks good. It would be nicer to match on device_type than on name,
but this is ancient code and it's probably best not to touch it
so we don't accidentally break some old SPARC or PPC system.

> */
>   compatible = "low-pin-count";
>   device_type = "isa";
>   #address-cells = <2>;
>   #size-cells = <1>;
>   reg = <0x0 0xa01b 0x0 0x1>;
>   ranges = <0x1 0x0 0x0 0x0 0x1000>;
> /*
> *  ranges is required, then i can get the IORESOURCE_IO <0xe4,4> from "reg = 
> <0x1, 0x00e4, 4>".
> *
> */
>   ipmi_0:ipmi@00e4{
>   device_type = "ipmi";
>   compatible = "ipmi-bt";
>   reg = <0x1 0x00e4 0x4>;
> };
> 

This looks wrong: the property above says that the I/O port range is
translated to MMIO address 0x to 0x0001, which is not
true on your hardware. I think this needs to be changed in the code
so the ranges property is not required for I/O ports.

> drivers\of\address.c
> static int __of_address_to_resource(struct device_node *dev,
>  const __be32 *addrp, u64 size, unsigned int flags,
>  const char *name, struct resource *r)
> {
>  u64 taddr;
> 
>  if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
>  return -EINVAL;
>  taddr = of_translate_address(dev, addrp);
>  if (taddr == OF_BAD_ADDR)
>  return -EINVAL;
>  memset(r, 0, sizeof(struct resource));
>  if (flags & IORESOURCE_IO) {
>  unsigned long port;
> 
> /*/
> /*legacy port(< 0x1000) is reserved, and need no translation here*/
> /*/
>  if(taddr + size < PCIBIOS_MIN_IO){
>  r->start = taddr;
>  r->end = taddr + size - 1;
>  }

I don't like having a special case based on the address here,
the same kind of hack might be needed for PCI I/O spaces in
hardware that uses an indirect method like your LPC bus
does, and the code above will not work on any LPC implementation
that correctly multiplexes its I/O ports with the first PCI domain.

I think it would be better to avoid translating the port into
a physical address to start with just to translate it back into
a port number, what we need instead is the offset between the
bus specific port number and the linux port number. I've added
Liviu to Cc, he wrote this code originally and may have some idea
of how we could do that.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Arnd Bergmann]

On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:
> 在 2015/12/31 23:00, Rongrong Zou 写道:
> > 2015-12-31 22:40 GMT+08:00 Arnd Bergmann  > >:
> >  > On Thursday 31 December 2015 22:12:19 Rongrong Zou wrote:
> >  > > 在 2015/12/30 17:06, Arnd Bergmann 写道:
> >  > > > On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
> >  >
> >  > The DT sample above looks good in principle. I believe what you are 
> > missing
> >  > here is code in your driver to scan the child nodes to create the 
> > platform
> >  > devices. of_bus_isa_translate() should work with your definition here
> >  > and create the correct IORESOURCE_IO resources. You don't have any MMIO
> >  > resources, so the absence of a ranges property is ok. Maybe all you
> >  > are missing is a call to of_platform_populate() or 
> > of_platform_bus_probe()?
> >  >
> >
> > You are right. thanks, i'll try on test board .  if i get the correct 
> > result , the new patch
> > will be sent later. By the way, it's my another email account use when i at 
> > home.
> 
> I tried, and there need some additional changes.
> 
> isa@a01b {
> 
> /*the node name should start with "isa", because of below definition
> * static int of_bus_isa_match(struct device_node *np)
> * {
> * return !strcmp(np->name, "isa");
> * }

Looks good. It would be nicer to match on device_type than on name,
but this is ancient code and it's probably best not to touch it
so we don't accidentally break some old SPARC or PPC system.

> */
>   compatible = "low-pin-count";
>   device_type = "isa";
>   #address-cells = <2>;
>   #size-cells = <1>;
>   reg = <0x0 0xa01b 0x0 0x1>;
>   ranges = <0x1 0x0 0x0 0x0 0x1000>;
> /*
> *  ranges is required, then i can get the IORESOURCE_IO <0xe4,4> from "reg = 
> <0x1, 0x00e4, 4>".
> *
> */
>   ipmi_0:ipmi@00e4{
>   device_type = "ipmi";
>   compatible = "ipmi-bt";
>   reg = <0x1 0x00e4 0x4>;
> };
> 

This looks wrong: the property above says that the I/O port range is
translated to MMIO address 0x to 0x0001, which is not
true on your hardware. I think this needs to be changed in the code
so the ranges property is not required for I/O ports.

> drivers\of\address.c
> static int __of_address_to_resource(struct device_node *dev,
>  const __be32 *addrp, u64 size, unsigned int flags,
>  const char *name, struct resource *r)
> {
>  u64 taddr;
> 
>  if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
>  return -EINVAL;
>  taddr = of_translate_address(dev, addrp);
>  if (taddr == OF_BAD_ADDR)
>  return -EINVAL;
>  memset(r, 0, sizeof(struct resource));
>  if (flags & IORESOURCE_IO) {
>  unsigned long port;
> 
> /*/
> /*legacy port(< 0x1000) is reserved, and need no translation here*/
> /*/
>  if(taddr + size < PCIBIOS_MIN_IO){
>  r->start = taddr;
>  r->end = taddr + size - 1;
>  }

I don't like having a special case based on the address here,
the same kind of hack might be needed for PCI I/O spaces in
hardware that uses an indirect method like your LPC bus
does, and the code above will not work on any LPC implementation
that correctly multiplexes its I/O ports with the first PCI domain.

I think it would be better to avoid translating the port into
a physical address to start with just to translate it back into
a port number, what we need instead is the offset between the
bus specific port number and the linux port number. I've added
Liviu to Cc, he wrote this code originally and may have some idea
of how we could do that.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

在 2016/1/4 19:13, Arnd Bergmann 写道:

On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:

在 2015/12/31 23:00, Rongrong Zou 写道:

2015-12-31 22:40 GMT+08:00 Arnd Bergmann >:
  > On Thursday 31 December 2015 22:12:19 Rongrong Zou wrote:
  > > 在 2015/12/30 17:06, Arnd Bergmann 写道:
  > > > On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
  >
  > The DT sample above looks good in principle. I believe what you are missing
  > here is code in your driver to scan the child nodes to create the platform
  > devices. of_bus_isa_translate() should work with your definition here
  > and create the correct IORESOURCE_IO resources. You don't have any MMIO
  > resources, so the absence of a ranges property is ok. Maybe all you
  > are missing is a call to of_platform_populate() or of_platform_bus_probe()?
  >

You are right. thanks, i'll try on test board .  if i get the correct result , 
the new patch
will be sent later. By the way, it's my another email account use when i at 
home.


I tried, and there need some additional changes.

isa@a01b {

/*the node name should start with "isa", because of below definition
* static int of_bus_isa_match(struct device_node *np)
* {
*   return !strcmp(np->name, "isa");
* }


Looks good. It would be nicer to match on device_type than on name,
but this is ancient code and it's probably best not to touch it
so we don't accidentally break some old SPARC or PPC system.


*/
compatible = "low-pin-count";
device_type = "isa";
#address-cells = <2>;
#size-cells = <1>;
reg = <0x0 0xa01b 0x0 0x1>;
ranges = <0x1 0x0 0x0 0x0 0x1000>;
/*
*  ranges is required, then i can get the IORESOURCE_IO <0xe4,4> from "reg = <0x1, 
0x00e4, 4>".
*
*/
ipmi_0:ipmi@00e4{
device_type = "ipmi";
compatible = "ipmi-bt";
reg = <0x1 0x00e4 0x4>;
};



This looks wrong: the property above says that the I/O port range is
translated to MMIO address 0x to 0x0001, which is not
true on your hardware. I think this needs to be changed in the code
so the ranges property is not required for I/O ports.


Ranges property can set empty, but this means 1:1 translation. the I/O 
port range is translated to MMIO address 0x0001  to 
0x0001 0004, it looks wrong else. I wonder if anyone get legacy 
I/O port resource from dts.


For ipmi driver, I can get I/O port resource by DMI rather than dts.




drivers\of\address.c
static int __of_address_to_resource(struct device_node *dev,
  const __be32 *addrp, u64 size, unsigned int flags,
  const char *name, struct resource *r)
{
  u64 taddr;

  if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
  return -EINVAL;
  taddr = of_translate_address(dev, addrp);
  if (taddr == OF_BAD_ADDR)
  return -EINVAL;
  memset(r, 0, sizeof(struct resource));
  if (flags & IORESOURCE_IO) {
  unsigned long port;

/*/
/*legacy port(< 0x1000) is reserved, and need no translation here*/
/*/
  if(taddr + size < PCIBIOS_MIN_IO){
  r->start = taddr;
  r->end = taddr + size - 1;
  }


I don't like having a special case based on the address here,
the same kind of hack might be needed for PCI I/O spaces in
hardware that uses an indirect method like your LPC bus
does, and the code above will not work on any LPC implementation
that correctly multiplexes its I/O ports with the first PCI domain.

I think it would be better to avoid translating the port into
a physical address to start with just to translate it back into
a port number, what we need instead is the offset between the
bus specific port number and the linux port number. I've added
Liviu to Cc, he wrote this code originally and may have some idea
of how we could do that.

Arnd


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Arnd Bergmann]

On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote:
> 在 2016/1/4 19:13, Arnd Bergmann 写道:
> > On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:
> >> 在 2015/12/31 23:00, Rongrong Zou 写道:
> >> */
> >>  compatible = "low-pin-count";
> >>  device_type = "isa";
> >>  #address-cells = <2>;
> >>  #size-cells = <1>;
> >>  reg = <0x0 0xa01b 0x0 0x1>;
> >>  ranges = <0x1 0x0 0x0 0x0 0x1000>;
> >> /*
> >> *  ranges is required, then i can get the IORESOURCE_IO <0xe4,4> from "reg 
> >> = <0x1, 0x00e4, 4>".
> >> *
> >> */
> >>  ipmi_0:ipmi@00e4{
> >>  device_type = "ipmi";
> >>  compatible = "ipmi-bt";
> >>  reg = <0x1 0x00e4 0x4>;
> >> };
> >>
> >
> > This looks wrong: the property above says that the I/O port range is
> > translated to MMIO address 0x to 0x0001, which is not
> > true on your hardware. I think this needs to be changed in the code
> > so the ranges property is not required for I/O ports.
> 
> Ranges property can set empty, but this means 1:1 translation. the I/O 
> port range is translated to MMIO address 0x0001  to 
> 0x0001 0004, it looks wrong else. I wonder if anyone get legacy 
> I/O port resource from dts.

As I said, nothing should really require the ranges property here, unless
you have a valid IORESOURCE_MEM translation. The code that requires
the ranges to be present is wrong.

> For ipmi driver, I can get I/O port resource by DMI rather than dts.

No, the ipmi driver uses the resource that belongs to the platform
device already, you can't rely on DMI data to be present there.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

在 2015/12/31 23:00, Rongrong Zou 写道:



2015-12-31 22:40 GMT+08:00 Arnd Bergmann mailto:a...@arndb.de>>:
 >
 > On Thursday 31 December 2015 22:12:19 Rongrong Zou wrote:
 > > 在 2015/12/30 17:06, Arnd Bergmann 写道:
 > > > On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
 > > >> +Example:
 > > >> +
 > > >> +lpc_0: lpc@a01b {
 > > >> +  #address-cells = <1>;
 > > >> +  #size-cells = <1>;
 > > >> +compatible = "low-pin-count";
 > > >> +reg = <0x0 0xa01b 0x0 0x1>;
 > > >> +};
 > > >
 > > > One more thought: please try to stick as closely as possible to the 
existing
 > > > ISA binding that is documented at
 > > >
 > > > http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
 > >  From the specification, I think I should use 2 32bit integer to describe 
the isa addr in dts.
 > > >
 > > > In particular, this should cover the possibility of describing both 
memory
 > > > and I/O spaces in child devices.
 > > >
 > >
 > > I found below config in powerpc dts "arch/powerpc/boot/dts/mpc8544ds.dts"
 > >
 > > isa@1e {
 > >  device_type = "isa";
 > >  #interrupt-cells = <2>;
 > >  #size-cells = <1>;
 > >  #address-cells = <2>;
 > >  reg = <0xf000 0x0 0x0 0x0 0x0>;
 > >  ranges = <0x1 0x0 0x100 0x0 0x0
 > >0x1000>;
 > >  interrupt-parent = <>;
 > >
 > >
 > >
 > >  rtc@70 {
 > >  compatible = "pnpPNP,b00";
 > >  reg = <0x1 0x70 0x2>;
 > >  };
 > >   the isa space in child-node: reg = <0x1 0x70 0x2>;
 > >   0x1 means IO space, 70 means addr, 0x2 is size.
 > >   but when i config the following in dts, the ipmi_0 node can't be probed,
 > >   I think there may be some problems.
 > >
 > > lpc_0: lpc@a01b {
 > >   compatible = "low-pin-count";
 > >   device_type = "isa";
 > >   #address-cells = <2>;
 > >   #size-cells = <1>;
 > >   reg = <0x0 0xa01b 0x0 0x1>;
 > >
 > >   ipmi_0:ipmi@00e4{
 > >   device_type = "ipmi";
 > >   compatible = "ipmi-bt";
 > >   reg = <0x1 0x00e4 0x4>;
 > > };
 >
 > The DT sample above looks good in principle. I believe what you are missing
 > here is code in your driver to scan the child nodes to create the platform
 > devices. of_bus_isa_translate() should work with your definition here
 > and create the correct IORESOURCE_IO resources. You don't have any MMIO
 > resources, so the absence of a ranges property is ok. Maybe all you
 > are missing is a call to of_platform_populate() or of_platform_bus_probe()?
 >

You are right. thanks, i'll try on test board .  if i get the correct result , 
the new patch
will be sent later. By the way, it's my another email account use when i at 
home.


I tried, and there need some additional changes.

isa@a01b {

/*the node name should start with "isa", because of below definition
* static int of_bus_isa_match(struct device_node *np)
* {
*   return !strcmp(np->name, "isa");
* }
*/
compatible = "low-pin-count";
device_type = "isa";
#address-cells = <2>;
#size-cells = <1>;
reg = <0x0 0xa01b 0x0 0x1>;
ranges = <0x1 0x0 0x0 0x0 0x1000>;
/*
*  ranges is required, then i can get the IORESOURCE_IO <0xe4,4> from "reg = <0x1, 
0x00e4, 4>".
*
*/
ipmi_0:ipmi@00e4{
device_type = "ipmi";
compatible = "ipmi-bt";
reg = <0x1 0x00e4 0x4>;
};

drivers\of\address.c
static int __of_address_to_resource(struct device_node *dev,
const __be32 *addrp, u64 size, unsigned int flags,
const char *name, struct resource *r)
{
u64 taddr;

if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
return -EINVAL;
taddr = of_translate_address(dev, addrp);
if (taddr == OF_BAD_ADDR)
return -EINVAL;
memset(r, 0, sizeof(struct resource));
if (flags & IORESOURCE_IO) {
unsigned long port;

/*/
/*legacy port(< 0x1000) is reserved, and need no translation here*/
/*/
if(taddr + size < PCIBIOS_MIN_IO){
r->start = taddr;
r->end = taddr + size - 1;
}


else{
port = pci_address_to_pio(taddr);
if (port == (unsigned long)-1)
return -EINVAL;
r->start = port;
  

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

在 2015/12/31 23:00, Rongrong Zou 写道:



2015-12-31 22:40 GMT+08:00 Arnd Bergmann >:
 >
 > On Thursday 31 December 2015 22:12:19 Rongrong Zou wrote:
 > > 在 2015/12/30 17:06, Arnd Bergmann 写道:
 > > > On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
 > > >> +Example:
 > > >> +
 > > >> +lpc_0: lpc@a01b {
 > > >> +  #address-cells = <1>;
 > > >> +  #size-cells = <1>;
 > > >> +compatible = "low-pin-count";
 > > >> +reg = <0x0 0xa01b 0x0 0x1>;
 > > >> +};
 > > >
 > > > One more thought: please try to stick as closely as possible to the 
existing
 > > > ISA binding that is documented at
 > > >
 > > > http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
 > >  From the specification, I think I should use 2 32bit integer to describe 
the isa addr in dts.
 > > >
 > > > In particular, this should cover the possibility of describing both 
memory
 > > > and I/O spaces in child devices.
 > > >
 > >
 > > I found below config in powerpc dts "arch/powerpc/boot/dts/mpc8544ds.dts"
 > >
 > > isa@1e {
 > >  device_type = "isa";
 > >  #interrupt-cells = <2>;
 > >  #size-cells = <1>;
 > >  #address-cells = <2>;
 > >  reg = <0xf000 0x0 0x0 0x0 0x0>;
 > >  ranges = <0x1 0x0 0x100 0x0 0x0
 > >0x1000>;
 > >  interrupt-parent = <>;
 > >
 > >
 > >
 > >  rtc@70 {
 > >  compatible = "pnpPNP,b00";
 > >  reg = <0x1 0x70 0x2>;
 > >  };
 > >   the isa space in child-node: reg = <0x1 0x70 0x2>;
 > >   0x1 means IO space, 70 means addr, 0x2 is size.
 > >   but when i config the following in dts, the ipmi_0 node can't be probed,
 > >   I think there may be some problems.
 > >
 > > lpc_0: lpc@a01b {
 > >   compatible = "low-pin-count";
 > >   device_type = "isa";
 > >   #address-cells = <2>;
 > >   #size-cells = <1>;
 > >   reg = <0x0 0xa01b 0x0 0x1>;
 > >
 > >   ipmi_0:ipmi@00e4{
 > >   device_type = "ipmi";
 > >   compatible = "ipmi-bt";
 > >   reg = <0x1 0x00e4 0x4>;
 > > };
 >
 > The DT sample above looks good in principle. I believe what you are missing
 > here is code in your driver to scan the child nodes to create the platform
 > devices. of_bus_isa_translate() should work with your definition here
 > and create the correct IORESOURCE_IO resources. You don't have any MMIO
 > resources, so the absence of a ranges property is ok. Maybe all you
 > are missing is a call to of_platform_populate() or of_platform_bus_probe()?
 >

You are right. thanks, i'll try on test board .  if i get the correct result , 
the new patch
will be sent later. By the way, it's my another email account use when i at 
home.


I tried, and there need some additional changes.

isa@a01b {

/*the node name should start with "isa", because of below definition
* static int of_bus_isa_match(struct device_node *np)
* {
*   return !strcmp(np->name, "isa");
* }
*/
compatible = "low-pin-count";
device_type = "isa";
#address-cells = <2>;
#size-cells = <1>;
reg = <0x0 0xa01b 0x0 0x1>;
ranges = <0x1 0x0 0x0 0x0 0x1000>;
/*
*  ranges is required, then i can get the IORESOURCE_IO <0xe4,4> from "reg = <0x1, 
0x00e4, 4>".
*
*/
ipmi_0:ipmi@00e4{
device_type = "ipmi";
compatible = "ipmi-bt";
reg = <0x1 0x00e4 0x4>;
};

drivers\of\address.c
static int __of_address_to_resource(struct device_node *dev,
const __be32 *addrp, u64 size, unsigned int flags,
const char *name, struct resource *r)
{
u64 taddr;

if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
return -EINVAL;
taddr = of_translate_address(dev, addrp);
if (taddr == OF_BAD_ADDR)
return -EINVAL;
memset(r, 0, sizeof(struct resource));
if (flags & IORESOURCE_IO) {
unsigned long port;

/*/
/*legacy port(< 0x1000) is reserved, and need no translation here*/
/*/
if(taddr + size < PCIBIOS_MIN_IO){
r->start = taddr;
r->end = taddr + size - 1;
}


else{
port = pci_address_to_pio(taddr);
if (port == (unsigned long)-1)
return -EINVAL;
r->start = port;
  

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Arnd Bergmann]

On Thursday 31 December 2015 22:12:19 Rongrong Zou wrote:
> 在 2015/12/30 17:06, Arnd Bergmann 写道:
> > On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
> >> +Example:
> >> +
> >> +lpc_0: lpc@a01b {
> >> +  #address-cells = <1>;
> >> +  #size-cells = <1>;
> >> +compatible = "low-pin-count";
> >> +reg = <0x0 0xa01b 0x0 0x1>;
> >> +};
> >
> > One more thought: please try to stick as closely as possible to the existing
> > ISA binding that is documented at
> >
> > http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
>  From the specification, I think I should use 2 32bit integer to describe the 
> isa addr in dts.
> >
> > In particular, this should cover the possibility of describing both memory
> > and I/O spaces in child devices.
> >
> 
> I found below config in powerpc dts "arch/powerpc/boot/dts/mpc8544ds.dts"
> 
> isa@1e {
>  device_type = "isa";
>  #interrupt-cells = <2>;
>  #size-cells = <1>;
>  #address-cells = <2>;
>  reg = <0xf000 0x0 0x0 0x0 0x0>;
>  ranges = <0x1 0x0 0x100 0x0 0x0
>0x1000>;
>  interrupt-parent = <>;
> 
> 
> 
>  rtc@70 {
>  compatible = "pnpPNP,b00";
>  reg = <0x1 0x70 0x2>;
>  };
>   the isa space in child-node: reg = <0x1 0x70 0x2>;
>   0x1 means IO space, 70 means addr, 0x2 is size.
>   but when i config the following in dts, the ipmi_0 node can't be probed,
>   I think there may be some problems.
> 
> lpc_0: lpc@a01b {
>   compatible = "low-pin-count";
>   device_type = "isa";
>   #address-cells = <2>;
>   #size-cells = <1>;
>   reg = <0x0 0xa01b 0x0 0x1>;
> 
>   ipmi_0:ipmi@00e4{
>   device_type = "ipmi";
>   compatible = "ipmi-bt";
>   reg = <0x1 0x00e4 0x4>;
> };

The DT sample above looks good in principle. I believe what you are missing
here is code in your driver to scan the child nodes to create the platform
devices. of_bus_isa_translate() should work with your definition here
and create the correct IORESOURCE_IO resources. You don't have any MMIO
resources, so the absence of a ranges property is ok. Maybe all you 
are missing is a call to of_platform_populate() or of_platform_bus_probe()?

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

Sorry for so late reply, it is difficult for me to understand ISA config :( .

在 2015/12/30 17:06, Arnd Bergmann 写道:

On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:

Signed-off-by: Rongrong Zou 
---
  .../devicetree/bindings/arm64/low-pin-count.txt  | 20 
  1 file changed, 20 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/arm64/low-pin-count.txt

diff --git a/Documentation/devicetree/bindings/arm64/low-pin-count.txt 
b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
new file mode 100644
index 000..215f2c4
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
@@ -0,0 +1,20 @@
+Low Pin Count bus driver
+
+Usually LPC controller is part of PCI host bridge, so the legacy ISA
+port locate on LPC bus can be accessed directly. But some SoC have
+independent LPC controller, and we can access the legacy port by specifying
+LPC address cycle. Thus, LPC driver is introduced.
+
+Required properties:
+- compatible: "low-pin-count"
+- reg: specifies low pin count address range
+
+
+Example:
+
+lpc_0: lpc@a01b {
+   #address-cells = <1>;
+   #size-cells = <1>;
+compatible = "low-pin-count";
+reg = <0x0 0xa01b 0x0 0x1>;
+};


One more thought: please try to stick as closely as possible to the existing
ISA binding that is documented at

http://www.firmware.org/1275/bindings/isa/isa0_4d.ps

From the specification, I think I should use 2 32bit integer to describe the 
isa addr in dts.


In particular, this should cover the possibility of describing both memory
and I/O spaces in child devices.



I found below config in powerpc dts "arch/powerpc/boot/dts/mpc8544ds.dts"

isa@1e {
device_type = "isa";
#interrupt-cells = <2>;
#size-cells = <1>;
#address-cells = <2>;
reg = <0xf000 0x0 0x0 0x0 0x0>;
ranges = <0x1 0x0 0x100 0x0 0x0
  0x1000>;
interrupt-parent = <>;



rtc@70 {
compatible = "pnpPNP,b00";
reg = <0x1 0x70 0x2>;
};
 the isa space in child-node: reg = <0x1 0x70 0x2>;
 0x1 means IO space, 70 means addr, 0x2 is size.
 but when i config the following in dts, the ipmi_0 node can't be probed,
 I think there may be some problems.

lpc_0: lpc@a01b {
compatible = "low-pin-count";
device_type = "isa";
#address-cells = <2>;
#size-cells = <1>;
reg = <0x0 0xa01b 0x0 0x1>;

ipmi_0:ipmi@00e4{
device_type = "ipmi";
compatible = "ipmi-bt";
reg = <0x1 0x00e4 0x4>;
};








Arnd
___
linuxarm mailing list
linux...@huawei.com
http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Arnd Bergmann]

On Thursday 31 December 2015 22:12:19 Rongrong Zou wrote:
> 在 2015/12/30 17:06, Arnd Bergmann 写道:
> > On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
> >> +Example:
> >> +
> >> +lpc_0: lpc@a01b {
> >> +  #address-cells = <1>;
> >> +  #size-cells = <1>;
> >> +compatible = "low-pin-count";
> >> +reg = <0x0 0xa01b 0x0 0x1>;
> >> +};
> >
> > One more thought: please try to stick as closely as possible to the existing
> > ISA binding that is documented at
> >
> > http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
>  From the specification, I think I should use 2 32bit integer to describe the 
> isa addr in dts.
> >
> > In particular, this should cover the possibility of describing both memory
> > and I/O spaces in child devices.
> >
> 
> I found below config in powerpc dts "arch/powerpc/boot/dts/mpc8544ds.dts"
> 
> isa@1e {
>  device_type = "isa";
>  #interrupt-cells = <2>;
>  #size-cells = <1>;
>  #address-cells = <2>;
>  reg = <0xf000 0x0 0x0 0x0 0x0>;
>  ranges = <0x1 0x0 0x100 0x0 0x0
>0x1000>;
>  interrupt-parent = <>;
> 
> 
> 
>  rtc@70 {
>  compatible = "pnpPNP,b00";
>  reg = <0x1 0x70 0x2>;
>  };
>   the isa space in child-node: reg = <0x1 0x70 0x2>;
>   0x1 means IO space, 70 means addr, 0x2 is size.
>   but when i config the following in dts, the ipmi_0 node can't be probed,
>   I think there may be some problems.
> 
> lpc_0: lpc@a01b {
>   compatible = "low-pin-count";
>   device_type = "isa";
>   #address-cells = <2>;
>   #size-cells = <1>;
>   reg = <0x0 0xa01b 0x0 0x1>;
> 
>   ipmi_0:ipmi@00e4{
>   device_type = "ipmi";
>   compatible = "ipmi-bt";
>   reg = <0x1 0x00e4 0x4>;
> };

The DT sample above looks good in principle. I believe what you are missing
here is code in your driver to scan the child nodes to create the platform
devices. of_bus_isa_translate() should work with your definition here
and create the correct IORESOURCE_IO resources. You don't have any MMIO
resources, so the absence of a ranges property is ok. Maybe all you 
are missing is a call to of_platform_populate() or of_platform_bus_probe()?

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

Sorry for so late reply, it is difficult for me to understand ISA config :( .

在 2015/12/30 17:06, Arnd Bergmann 写道:

On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:

Signed-off-by: Rongrong Zou 
---
  .../devicetree/bindings/arm64/low-pin-count.txt  | 20 
  1 file changed, 20 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/arm64/low-pin-count.txt

diff --git a/Documentation/devicetree/bindings/arm64/low-pin-count.txt 
b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
new file mode 100644
index 000..215f2c4
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
@@ -0,0 +1,20 @@
+Low Pin Count bus driver
+
+Usually LPC controller is part of PCI host bridge, so the legacy ISA
+port locate on LPC bus can be accessed directly. But some SoC have
+independent LPC controller, and we can access the legacy port by specifying
+LPC address cycle. Thus, LPC driver is introduced.
+
+Required properties:
+- compatible: "low-pin-count"
+- reg: specifies low pin count address range
+
+
+Example:
+
+lpc_0: lpc@a01b {
+   #address-cells = <1>;
+   #size-cells = <1>;
+compatible = "low-pin-count";
+reg = <0x0 0xa01b 0x0 0x1>;
+};


One more thought: please try to stick as closely as possible to the existing
ISA binding that is documented at

http://www.firmware.org/1275/bindings/isa/isa0_4d.ps

From the specification, I think I should use 2 32bit integer to describe the 
isa addr in dts.


In particular, this should cover the possibility of describing both memory
and I/O spaces in child devices.



I found below config in powerpc dts "arch/powerpc/boot/dts/mpc8544ds.dts"

isa@1e {
device_type = "isa";
#interrupt-cells = <2>;
#size-cells = <1>;
#address-cells = <2>;
reg = <0xf000 0x0 0x0 0x0 0x0>;
ranges = <0x1 0x0 0x100 0x0 0x0
  0x1000>;
interrupt-parent = <>;



rtc@70 {
compatible = "pnpPNP,b00";
reg = <0x1 0x70 0x2>;
};
 the isa space in child-node: reg = <0x1 0x70 0x2>;
 0x1 means IO space, 70 means addr, 0x2 is size.
 but when i config the following in dts, the ipmi_0 node can't be probed,
 I think there may be some problems.

lpc_0: lpc@a01b {
compatible = "low-pin-count";
device_type = "isa";
#address-cells = <2>;
#size-cells = <1>;
reg = <0x0 0xa01b 0x0 0x1>;

ipmi_0:ipmi@00e4{
device_type = "ipmi";
compatible = "ipmi-bt";
reg = <0x1 0x00e4 0x4>;
};








Arnd
___
linuxarm mailing list
linux...@huawei.com
http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Arnd Bergmann]

On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
> Signed-off-by: Rongrong Zou 
> ---
>  .../devicetree/bindings/arm64/low-pin-count.txt  | 20 
> 
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm64/low-pin-count.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm64/low-pin-count.txt 
> b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
> new file mode 100644
> index 000..215f2c4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
> @@ -0,0 +1,20 @@
> +Low Pin Count bus driver
> +
> +Usually LPC controller is part of PCI host bridge, so the legacy ISA
> +port locate on LPC bus can be accessed directly. But some SoC have
> +independent LPC controller, and we can access the legacy port by specifying
> +LPC address cycle. Thus, LPC driver is introduced.
> +
> +Required properties:
> +- compatible: "low-pin-count"
> +- reg: specifies low pin count address range
> +
> +
> +Example:
> +
> +lpc_0: lpc@a01b {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +compatible = "low-pin-count";
> +reg = <0x0 0xa01b 0x0 0x1>;
> +};

One more thought: please try to stick as closely as possible to the existing
ISA binding that is documented at

http://www.firmware.org/1275/bindings/isa/isa0_4d.ps

In particular, this should cover the possibility of describing both memory
and I/O spaces in child devices.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Arnd Bergmann]

On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
> Signed-off-by: Rongrong Zou 
> ---
>  .../devicetree/bindings/arm64/low-pin-count.txt  | 20 
> 
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm64/low-pin-count.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm64/low-pin-count.txt 
> b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
> new file mode 100644
> index 000..215f2c4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
> @@ -0,0 +1,20 @@
> +Low Pin Count bus driver
> +
> +Usually LPC controller is part of PCI host bridge, so the legacy ISA
> +port locate on LPC bus can be accessed directly. But some SoC have
> +independent LPC controller, and we can access the legacy port by specifying
> +LPC address cycle. Thus, LPC driver is introduced.
> +
> +Required properties:
> +- compatible: "low-pin-count"
> +- reg: specifies low pin count address range
> +
> +
> +Example:
> +
> +lpc_0: lpc@a01b {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +compatible = "low-pin-count";
> +reg = <0x0 0xa01b 0x0 0x1>;
> +};

One more thought: please try to stick as closely as possible to the existing
ISA binding that is documented at

http://www.firmware.org/1275/bindings/isa/isa0_4d.ps

In particular, this should cover the possibility of describing both memory
and I/O spaces in child devices.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Arnd Bergmann]

On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
> Signed-off-by: Rongrong Zou 
> ---
>  .../devicetree/bindings/arm64/low-pin-count.txt  | 20 
> 
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm64/low-pin-count.txt

Please add a patch description above and Cc the devicetree mailing list
when you submit it again.

> diff --git a/Documentation/devicetree/bindings/arm64/low-pin-count.txt 
> b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
> new file mode 100644
> index 000..215f2c4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
> @@ -0,0 +1,20 @@
> +Low Pin Count bus driver
> +
> +Usually LPC controller is part of PCI host bridge, so the legacy ISA
> +port locate on LPC bus can be accessed directly. But some SoC have
> +independent LPC controller, and we can access the legacy port by specifying
> +LPC address cycle. Thus, LPC driver is introduced.
> +
> +Required properties:
> +- compatible: "low-pin-count"
> +- reg: specifies low pin count address range
> +

It would be good to add an explanation about the address space for child
devices.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

Signed-off-by: Rongrong Zou 
---
 .../devicetree/bindings/arm64/low-pin-count.txt  | 20 
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm64/low-pin-count.txt

diff --git a/Documentation/devicetree/bindings/arm64/low-pin-count.txt 
b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
new file mode 100644
index 000..215f2c4
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
@@ -0,0 +1,20 @@
+Low Pin Count bus driver
+
+Usually LPC controller is part of PCI host bridge, so the legacy ISA
+port locate on LPC bus can be accessed directly. But some SoC have
+independent LPC controller, and we can access the legacy port by specifying
+LPC address cycle. Thus, LPC driver is introduced.
+
+Required properties:
+- compatible: "low-pin-count"
+- reg: specifies low pin count address range
+
+
+Example:
+
+lpc_0: lpc@a01b {
+   #address-cells = <1>;
+   #size-cells = <1>;
+compatible = "low-pin-count";
+reg = <0x0 0xa01b 0x0 0x1>;
+};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

[Arnd Bergmann]

On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
> Signed-off-by: Rongrong Zou 
> ---
>  .../devicetree/bindings/arm64/low-pin-count.txt  | 20 
> 
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm64/low-pin-count.txt

Please add a patch description above and Cc the devicetree mailing list
when you submit it again.

> diff --git a/Documentation/devicetree/bindings/arm64/low-pin-count.txt 
> b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
> new file mode 100644
> index 000..215f2c4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
> @@ -0,0 +1,20 @@
> +Low Pin Count bus driver
> +
> +Usually LPC controller is part of PCI host bridge, so the legacy ISA
> +port locate on LPC bus can be accessed directly. But some SoC have
> +independent LPC controller, and we can access the legacy port by specifying
> +LPC address cycle. Thus, LPC driver is introduced.
> +
> +Required properties:
> +- compatible: "low-pin-count"
> +- reg: specifies low pin count address range
> +

It would be good to add an explanation about the address space for child
devices.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v1 3/3] ARM64 LPC: update binding doc

[Rongrong Zou]

Signed-off-by: Rongrong Zou 
---
 .../devicetree/bindings/arm64/low-pin-count.txt  | 20 
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm64/low-pin-count.txt

diff --git a/Documentation/devicetree/bindings/arm64/low-pin-count.txt 
b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
new file mode 100644
index 000..215f2c4
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
@@ -0,0 +1,20 @@
+Low Pin Count bus driver
+
+Usually LPC controller is part of PCI host bridge, so the legacy ISA
+port locate on LPC bus can be accessed directly. But some SoC have
+independent LPC controller, and we can access the legacy port by specifying
+LPC address cycle. Thus, LPC driver is introduced.
+
+Required properties:
+- compatible: "low-pin-count"
+- reg: specifies low pin count address range
+
+
+Example:
+
+lpc_0: lpc@a01b {
+   #address-cells = <1>;
+   #size-cells = <1>;
+compatible = "low-pin-count";
+reg = <0x0 0xa01b 0x0 0x1>;
+};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/