RE: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings

2017-11-09 Thread Gabriele Paoloni 

Hi Dann

[...]

> > +
> > +#define START_WORK 0x01
> 
> Any reason not to put this in the LPC_ namespace as well?

No, not really. We'll make it consistent in the next patchset 

> 
> > +/* The minimal nanosecond interval for each query on LPC cycle
> status. */

[...]

> > + * hisilpc_target_in - trigger a series of lpc cycles to read
> required data
> > + *from target peripheral.
> > + * @pdev: pointer to hisi lpc device
> 
> It's now lpcdev
> 

Thanks, we'll change it.

> > + * @para: some parameters used to control the lpc I/O operations

[...]

> > + * @pio: the target I/O port address.
> 
> @outval & @pio are in the opposite order of the actual function

Thanks, we'll change it.

> 

[...]

> > +   lpcdev->io_host->devpara = NULL;
> > +   dev_err(dev, "OF: scan hisilpc children got failed(%d)\n",
> > +   ret);
> 
> nit: Maybe "OF: scanning hisilpc children failed(%d)" ?
> 

Yes, thanks we'll change it.

> > +   return ret;
> > +   }
> > +
> > +   dev_info(dev, "hslpc end probing. range[%pa - sz:%pa]\n",
> > +   >io_host->io_start,
> > +   >io_host->size);
> > +
> > +   return ret;
> > +}
> 
> 
>  -dann

RE: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings

2017-11-09 Thread Gabriele Paoloni 

Hi Dann

[...]

> > +
> > +#define START_WORK 0x01
> 
> Any reason not to put this in the LPC_ namespace as well?

No, not really. We'll make it consistent in the next patchset 

> 
> > +/* The minimal nanosecond interval for each query on LPC cycle
> status. */

[...]

> > + * hisilpc_target_in - trigger a series of lpc cycles to read
> required data
> > + *from target peripheral.
> > + * @pdev: pointer to hisi lpc device
> 
> It's now lpcdev
> 

Thanks, we'll change it.

> > + * @para: some parameters used to control the lpc I/O operations

[...]

> > + * @pio: the target I/O port address.
> 
> @outval & @pio are in the opposite order of the actual function

Thanks, we'll change it.

> 

[...]

> > +   lpcdev->io_host->devpara = NULL;
> > +   dev_err(dev, "OF: scan hisilpc children got failed(%d)\n",
> > +   ret);
> 
> nit: Maybe "OF: scanning hisilpc children failed(%d)" ?
> 

Yes, thanks we'll change it.

> > +   return ret;
> > +   }
> > +
> > +   dev_info(dev, "hslpc end probing. range[%pa - sz:%pa]\n",
> > +   >io_host->io_start,
> > +   >io_host->size);
> > +
> > +   return ret;
> > +}
> 
> 
>  -dann

Re: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings

2017-11-08 Thread dann frazier 
On Fri, Oct 27, 2017 at 05:11:24PM +0100, Gabriele Paoloni wrote:
> From: "zhichang.yuan" 
> 
> The low-pin-count(LPC) interface of Hip06/Hip07 accesses the peripherals in
> I/O port addresses. This patch implements the LPC host controller driver
> which perform the I/O operations on the underlying hardware.
> We don't want to touch those existing peripherals' driver, such as ipmi-bt.
> So this driver applies the indirect-IO introduced in the previous patch
> after registering an indirect-IO node to the indirect-IO devices list which
> will be searched in the I/O accessors to retrieve the host-local I/O port.
> 
> Signed-off-by: Zou Rongrong 
> Signed-off-by: zhichang.yuan 
> Signed-off-by: Gabriele Paoloni 
> Acked-by: Rob Herring  #dts part
> ---
>  .../arm/hisilicon/hisilicon-low-pin-count.txt  |  33 ++
>  drivers/bus/Kconfig|   9 +
>  drivers/bus/Makefile   |   1 +
>  drivers/bus/hisi_lpc.c | 526 
> +
>  4 files changed, 569 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
>  create mode 100644 drivers/bus/hisi_lpc.c
> 
> 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 000..213181f
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
> @@ -0,0 +1,33 @@
> +Hisilicon Hip06 low-pin-count device
> +  Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, which
> +  provides I/O access to some legacy ISA devices.
> +  Hip06 is based on arm64 architecture where there is no I/O space. So, the
> +  I/O ports here are not cpu addresses, and there is no 'ranges' property in
> +  LPC device node.
> +
> +Required properties:
> +- compatible:  value should be as follows:
> + (a) "hisilicon,hip06-lpc"
> + (b) "hisilicon,hip07-lpc"
> +- #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 memory range where the LPC register set is mapped.
> +
> +Note:
> +  The node name before '@' must be "isa" to represent the binding stick to 
> the
> +  ISA/EISA binding specification.
> +
> +Example:
> +
> +isa@a01b {
> + compatible = "hisilicon,hip06-lpc";
> + #address-cells = <2>;
> + #size-cells = <1>;
> + reg = <0x0 0xa01b 0x0 0x1000>;
> +
> + ipmi0: bt@e4 {
> + compatible = "ipmi-bt";
> + device_type = "ipmi";
> + reg = <0x01 0xe4 0x04>;
> + };
> +};
> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> index 2408ea3..358eed3 100644
> --- a/drivers/bus/Kconfig
> +++ b/drivers/bus/Kconfig
> @@ -64,6 +64,15 @@ config BRCMSTB_GISB_ARB
> arbiter. This driver provides timeout and target abort error handling
> and internal bus master decoding.
>  
> +config HISILICON_LPC
> + bool "Support for ISA I/O space on Hisilicon Hip0X"
> + depends on (ARM64 && (ARCH_HISI || COMPILE_TEST))
> + select LOGIC_PIO
> + select 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 cc6364b..28e3862 100644
> --- a/drivers/bus/Makefile
> +++ b/drivers/bus/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_ARM_CCI) += arm-cci.o
>  obj-$(CONFIG_ARM_CCN)+= arm-ccn.o
>  
>  obj-$(CONFIG_BRCMSTB_GISB_ARB)   += brcmstb_gisb.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 000..c885483
> --- /dev/null
> +++ b/drivers/bus/hisi_lpc.c
> @@ -0,0 +1,526 @@
> +/*
> + * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved.
> + * Author: Zhichang Yuan 
> + * Author: Zou Rongrong 
> + *
> + * 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

Re: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings

2017-11-08 Thread dann frazier 
On Fri, Oct 27, 2017 at 05:11:24PM +0100, Gabriele Paoloni wrote:
> From: "zhichang.yuan" 
> 
> The low-pin-count(LPC) interface of Hip06/Hip07 accesses the peripherals in
> I/O port addresses. This patch implements the LPC host controller driver
> which perform the I/O operations on the underlying hardware.
> We don't want to touch those existing peripherals' driver, such as ipmi-bt.
> So this driver applies the indirect-IO introduced in the previous patch
> after registering an indirect-IO node to the indirect-IO devices list which
> will be searched in the I/O accessors to retrieve the host-local I/O port.
> 
> Signed-off-by: Zou Rongrong 
> Signed-off-by: zhichang.yuan 
> Signed-off-by: Gabriele Paoloni 
> Acked-by: Rob Herring  #dts part
> ---
>  .../arm/hisilicon/hisilicon-low-pin-count.txt  |  33 ++
>  drivers/bus/Kconfig|   9 +
>  drivers/bus/Makefile   |   1 +
>  drivers/bus/hisi_lpc.c | 526 
> +
>  4 files changed, 569 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
>  create mode 100644 drivers/bus/hisi_lpc.c
> 
> 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 000..213181f
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
> @@ -0,0 +1,33 @@
> +Hisilicon Hip06 low-pin-count device
> +  Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, which
> +  provides I/O access to some legacy ISA devices.
> +  Hip06 is based on arm64 architecture where there is no I/O space. So, the
> +  I/O ports here are not cpu addresses, and there is no 'ranges' property in
> +  LPC device node.
> +
> +Required properties:
> +- compatible:  value should be as follows:
> + (a) "hisilicon,hip06-lpc"
> + (b) "hisilicon,hip07-lpc"
> +- #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 memory range where the LPC register set is mapped.
> +
> +Note:
> +  The node name before '@' must be "isa" to represent the binding stick to 
> the
> +  ISA/EISA binding specification.
> +
> +Example:
> +
> +isa@a01b {
> + compatible = "hisilicon,hip06-lpc";
> + #address-cells = <2>;
> + #size-cells = <1>;
> + reg = <0x0 0xa01b 0x0 0x1000>;
> +
> + ipmi0: bt@e4 {
> + compatible = "ipmi-bt";
> + device_type = "ipmi";
> + reg = <0x01 0xe4 0x04>;
> + };
> +};
> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> index 2408ea3..358eed3 100644
> --- a/drivers/bus/Kconfig
> +++ b/drivers/bus/Kconfig
> @@ -64,6 +64,15 @@ config BRCMSTB_GISB_ARB
> arbiter. This driver provides timeout and target abort error handling
> and internal bus master decoding.
>  
> +config HISILICON_LPC
> + bool "Support for ISA I/O space on Hisilicon Hip0X"
> + depends on (ARM64 && (ARCH_HISI || COMPILE_TEST))
> + select LOGIC_PIO
> + select 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 cc6364b..28e3862 100644
> --- a/drivers/bus/Makefile
> +++ b/drivers/bus/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_ARM_CCI) += arm-cci.o
>  obj-$(CONFIG_ARM_CCN)+= arm-ccn.o
>  
>  obj-$(CONFIG_BRCMSTB_GISB_ARB)   += brcmstb_gisb.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 000..c885483
> --- /dev/null
> +++ b/drivers/bus/hisi_lpc.c
> @@ -0,0 +1,526 @@
> +/*
> + * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved.
> + * Author: Zhichang Yuan 
> + * Author: Zou Rongrong 
> + *
> + * 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 .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
>

RE: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings

2017-10-30 Thread Gabriele Paoloni 
Hi Randy

> -Original Message-
> From: Randy Dunlap [mailto:rdun...@infradead.org]
> Sent: 27 October 2017 17:44
> To: Gabriele Paoloni; catalin.mari...@arm.com; will.dea...@arm.com;
> robh...@kernel.org; frowand.l...@gmail.com; bhelg...@google.com;
> raf...@kernel.org; a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> lorenzo.pieral...@arm.com
> Cc: mark.rutl...@arm.com; brian.star...@arm.com; o...@lixom.net;
> b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux-
> a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org;
> miny...@acm.org; John Garry; xuwei (O); zhichang.yuan
> Subject: Re: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07
> with DT bindings
> 
> On 10/27/17 09:11, Gabriele Paoloni wrote:
> > diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> > index 2408ea3..358eed3 100644
> > --- a/drivers/bus/Kconfig
> > +++ b/drivers/bus/Kconfig
> > @@ -64,6 +64,15 @@ config BRCMSTB_GISB_ARB
> >   arbiter. This driver provides timeout and target abort error
> handling
> >   and internal bus master decoding.
> >
> > +config HISILICON_LPC
> > +   bool "Support for ISA I/O space on Hisilicon Hip0X"
> > +   depends on (ARM64 && (ARCH_HISI || COMPILE_TEST))
> > +   select LOGIC_PIO
> > +   select 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
> 
> Hi,
> 
> Why bool? why not tristate?

Well for the nature of our HW it would not make much sense to have the
LPC modular. Also you can see in patch 8 the LPC host is "translating"
the resources of its children before these are actually probed and this
is done by acpi_indirectio_scan_init() as part of the ACPI init process.

Thanks
Gab

> 
> --
> ~Randy

RE: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings

2017-10-30 Thread Gabriele Paoloni 
Hi Randy

> -Original Message-
> From: Randy Dunlap [mailto:rdun...@infradead.org]
> Sent: 27 October 2017 17:44
> To: Gabriele Paoloni; catalin.mari...@arm.com; will.dea...@arm.com;
> robh...@kernel.org; frowand.l...@gmail.com; bhelg...@google.com;
> raf...@kernel.org; a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> lorenzo.pieral...@arm.com
> Cc: mark.rutl...@arm.com; brian.star...@arm.com; o...@lixom.net;
> b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux-
> a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org;
> miny...@acm.org; John Garry; xuwei (O); zhichang.yuan
> Subject: Re: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07
> with DT bindings
> 
> On 10/27/17 09:11, Gabriele Paoloni wrote:
> > diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> > index 2408ea3..358eed3 100644
> > --- a/drivers/bus/Kconfig
> > +++ b/drivers/bus/Kconfig
> > @@ -64,6 +64,15 @@ config BRCMSTB_GISB_ARB
> >   arbiter. This driver provides timeout and target abort error
> handling
> >   and internal bus master decoding.
> >
> > +config HISILICON_LPC
> > +   bool "Support for ISA I/O space on Hisilicon Hip0X"
> > +   depends on (ARM64 && (ARCH_HISI || COMPILE_TEST))
> > +   select LOGIC_PIO
> > +   select 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
> 
> Hi,
> 
> Why bool? why not tristate?

Well for the nature of our HW it would not make much sense to have the
LPC modular. Also you can see in patch 8 the LPC host is "translating"
the resources of its children before these are actually probed and this
is done by acpi_indirectio_scan_init() as part of the ACPI init process.

Thanks
Gab

> 
> --
> ~Randy

Re: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings

2017-10-27 Thread Randy Dunlap 
On 10/27/17 09:11, Gabriele Paoloni wrote:
> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> index 2408ea3..358eed3 100644
> --- a/drivers/bus/Kconfig
> +++ b/drivers/bus/Kconfig
> @@ -64,6 +64,15 @@ config BRCMSTB_GISB_ARB
> arbiter. This driver provides timeout and target abort error handling
> and internal bus master decoding.
>  
> +config HISILICON_LPC
> + bool "Support for ISA I/O space on Hisilicon Hip0X"
> + depends on (ARM64 && (ARCH_HISI || COMPILE_TEST))
> + select LOGIC_PIO
> + select 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

Hi,

Why bool? why not tristate?

-- 
~Randy

Re: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings

2017-10-27 Thread Randy Dunlap 
On 10/27/17 09:11, Gabriele Paoloni wrote:
> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> index 2408ea3..358eed3 100644
> --- a/drivers/bus/Kconfig
> +++ b/drivers/bus/Kconfig
> @@ -64,6 +64,15 @@ config BRCMSTB_GISB_ARB
> arbiter. This driver provides timeout and target abort error handling
> and internal bus master decoding.
>  
> +config HISILICON_LPC
> + bool "Support for ISA I/O space on Hisilicon Hip0X"
> + depends on (ARM64 && (ARCH_HISI || COMPILE_TEST))
> + select LOGIC_PIO
> + select 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

Hi,

Why bool? why not tristate?

-- 
~Randy