Re: [PATCH v5 3/3] dt/bindings: qcom_nandc: Add DT bindings

2016-01-06 Thread Boris Brezillon 
Hi Archit,

On Tue,  5 Jan 2016 10:55:01 +0530
Archit Taneja  wrote:

> Add DT bindings document for the Qualcomm NAND controller driver.
> 
> Cc: devicet...@vger.kernel.org
> Cc: Rob Herring 
> Signed-off-by: Archit Taneja 
> ---
> v5:
> - Make changes to incorporate chip select sub nodes (brcmnand taken as
>   reference)
> 
> v3:
> - Don't use '0x' when specifying nand controller address space
> - Add optional property for on-flash bbt usage
> 
>  .../devicetree/bindings/mtd/qcom_nandc.txt | 84 
> ++
>  1 file changed, 84 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt 
> b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> new file mode 100644
> index 000..b2cf2d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> @@ -0,0 +1,84 @@
> +* Qualcomm NAND controller
> +
> +Required properties:
> +- compatible:should be "qcom,ebi2-nand" for IPQ806x
> +- reg:   MMIO address range
> +- clocks:must contain core clock and always on clock
> +- clock-names:   must contain "core" for the core clock and 
> "aon" for the
> + always on clock
> +- dmas:  DMA specifier, consisting of a phandle to the 
> ADM DMA
> + controller node and the channel number to be used for
> + NAND. Refer to dma.txt and qcom_adm.txt for more details
> +- dma-names: must be "rxtx"
> +- qcom,cmd-crci: must contain the ADM command type CRCI block instance
> + number specified for the NAND controller on the given
> + platform
> +- qcom,data-crci:must contain the ADM data type CRCI block instance
> + number specified for the NAND controller on the given
> + platform
> +- #address-cells:<1> - subnodes give the chip-select number
> +- #size-cells:   <0>
> +
> +* NAND chip-select
> +
> +Each controller may contain one or more subnodes to represent enabled
> +chip-selects which (may) contain NAND flash chips. Their properties are as
> +follows.
> +
> +Required properties:
> +- compatible:should contain "qcom,nandcs"
> +- reg:   a single integer representing the chip-select
> + number (e.g., 0, 1, 2, etc.)
> +- #address-cells:see partition.txt
> +- #size-cells:   see partition.txt
> +- nand-ecc-strength: number of bits to correct per ECC step. Must be 4 or 8
> + bits.
> +- nand-ecc-step-size:bytes of data per ECC step. Must be 512.
> +
> +Optional properties:
> +- nand-bus-width:bus width. Must be 8 or 16. If not present, 8 is chosen
> + as default

You should probably reference
Documentation/devicetree/bindings/mtd/nand.txt which is documenting
generic nand-xxx properties.

> +
> +Each nandcs device node may optionally contain sub-nodes describing the flash
> +partition mapping. See partition.txt for more detail.
> +
> +Example:
> +
> +nand@1ac0 {
> + compatible = "qcom,ebi2-nandc";
> + reg = <0x1ac0 0x800>;
> +
> + clocks = < EBI2_CLK>,
> +  < EBI2_AON_CLK>;
> + clock-names = "core", "aon";
> +
> + dmas = <_dma 3>;
> + dma-names = "rxtx";
> + qcom,cmd-crci = <15>;
> + qcom,data-crci = <3>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + nandcs@0 {
> + compatible = "qcom,nandcs";
> + reg = <0>;
> +
> + nand-ecc-strength = <4>;
> + nand-ecc-step-size = <512>;
> + nand-bus-width = <8>;
> +

It's now recommended to define a 'partitions' subnode to store those
partition nodes.

> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + partition@0 {
> + label = "boot-nand";
> + reg = <0 0x58a>;
> + };
> +
> + partition@58a {
> + label = "fs-nand";
> + reg = <0x58a 0x400>;
> + };
> + };
> +};

The rest looks good to me.

Reviewed-by: Boris Brezillon 

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 3/3] dt/bindings: qcom_nandc: Add DT bindings

2016-01-06 Thread Rob Herring 
On Tue, Jan 05, 2016 at 10:55:01AM +0530, Archit Taneja wrote:
> Add DT bindings document for the Qualcomm NAND controller driver.
> 
> Cc: devicet...@vger.kernel.org
> Cc: Rob Herring 
> Signed-off-by: Archit Taneja 
> ---
> v5:
> - Make changes to incorporate chip select sub nodes (brcmnand taken as
>   reference)
> 
> v3:
> - Don't use '0x' when specifying nand controller address space
> - Add optional property for on-flash bbt usage
> 
>  .../devicetree/bindings/mtd/qcom_nandc.txt | 84 
> ++
>  1 file changed, 84 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt 
> b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> new file mode 100644
> index 000..b2cf2d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> @@ -0,0 +1,84 @@
> +* Qualcomm NAND controller
> +
> +Required properties:
> +- compatible:should be "qcom,ebi2-nand" for IPQ806x

More specific name please.

> +- reg:   MMIO address range
> +- clocks:must contain core clock and always on clock
> +- clock-names:   must contain "core" for the core clock and 
> "aon" for the
> + always on clock
> +- dmas:  DMA specifier, consisting of a phandle to the 
> ADM DMA
> + controller node and the channel number to be used for
> + NAND. Refer to dma.txt and qcom_adm.txt for more details
> +- dma-names: must be "rxtx"
> +- qcom,cmd-crci: must contain the ADM command type CRCI block instance
> + number specified for the NAND controller on the given
> + platform
> +- qcom,data-crci:must contain the ADM data type CRCI block instance
> + number specified for the NAND controller on the given
> + platform
> +- #address-cells:<1> - subnodes give the chip-select number
> +- #size-cells:   <0>
> +
> +* NAND chip-select
> +
> +Each controller may contain one or more subnodes to represent enabled
> +chip-selects which (may) contain NAND flash chips. Their properties are as
> +follows.
> +
> +Required properties:
> +- compatible:should contain "qcom,nandcs"
> +- reg:   a single integer representing the chip-select
> + number (e.g., 0, 1, 2, etc.)
> +- #address-cells:see partition.txt
> +- #size-cells:   see partition.txt
> +- nand-ecc-strength: number of bits to correct per ECC step. Must be 4 or 8
> + bits.
> +- nand-ecc-step-size:bytes of data per ECC step. Must be 512.
> +
> +Optional properties:
> +- nand-bus-width:bus width. Must be 8 or 16. If not present, 8 is chosen
> + as default
> +
> +Each nandcs device node may optionally contain sub-nodes describing the flash
> +partition mapping. See partition.txt for more detail.

Can't the partitioning span across chip selects? After all, interleaving 
is how you get high performance.

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

Re: [PATCH v5 1/3] mtd: nand: don't select chip in nand_chip's block_bad op

2016-01-06 Thread Boris Brezillon 
On Tue,  5 Jan 2016 10:54:59 +0530
Archit Taneja  wrote:

> One of the arguments passed to struct nand_chip's block_bad op is
> 'getchip', which, if true, is supposed to get and select the nand device,
> and later unselect and release the device.
> 
> This op is intended to be replaceable by drivers. The drivers shouldn't
> be responsible for selecting/unselecting chip. Like other ops, the chip
> should already be selected before the block_bad op is called.
> 
> Remove the getchip argument from the block_bad op and move the chip
> selection to nand_block_checkbad (the only user of chip->block_bad).
> Modify nand_block_bad (the default function for the op) such that
> it doesn't select the chip.

I would go even further and move the chip selection into
nand_block_isbad(), who is the only caller that requires this chip
selection.

> 
> Signed-off-by: Archit Taneja 
> ---
>  drivers/mtd/nand/nand_base.c | 41 +++--
>  include/linux/mtd/nand.h |  2 +-
>  2 files changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 928081b..42aa441 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -317,9 +317,9 @@ static void nand_read_buf16(struct mtd_info *mtd, uint8_t 
> *buf, int len)
>   *
>   * Check, if the block is bad.
>   */
> -static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
> +static int nand_block_bad(struct mtd_info *mtd, loff_t ofs)
>  {
> - int page, chipnr, res = 0, i = 0;
> + int page, res = 0, i = 0;
>   struct nand_chip *chip = mtd_to_nand(mtd);
>   u16 bad;
>  
> @@ -328,15 +328,6 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t 
> ofs, int getchip)
>  
>   page = (int)(ofs >> chip->page_shift) & chip->pagemask;
>  
> - if (getchip) {
> - chipnr = (int)(ofs >> chip->chip_shift);
> -
> - nand_get_device(mtd, FL_READING);
> -
> - /* Select the NAND device */
> - chip->select_chip(mtd, chipnr);
> - }
> -
>   do {
>   if (chip->options & NAND_BUSWIDTH_16) {
>   chip->cmdfunc(mtd, NAND_CMD_READOOB,
> @@ -361,11 +352,6 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t 
> ofs, int getchip)
>   i++;
>   } while (!res && i < 2 && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE));
>  
> - if (getchip) {
> - chip->select_chip(mtd, -1);
> - nand_release_device(mtd);
> - }
> -
>   return res;
>  }
>  
> @@ -514,8 +500,27 @@ static int nand_block_checkbad(struct mtd_info *mtd, 
> loff_t ofs, int getchip,
>  {
>   struct nand_chip *chip = mtd_to_nand(mtd);
>  
> - if (!chip->bbt)
> - return chip->block_bad(mtd, ofs, getchip);
> + if (!chip->bbt) {
> + int ret;
> +
> + if (getchip) {
> + int chipnr = (int)(ofs >> chip->chip_shift);
> +
> + nand_get_device(mtd, FL_READING);
> +
> + /* Select the NAND device */
> + chip->select_chip(mtd, chipnr);
> + }
> +
> + ret = chip->block_bad(mtd, ofs);
> +
> + if (getchip) {
> + chip->select_chip(mtd, -1);
> + nand_release_device(mtd);
> + }
> +
> + return ret;
> + }
>  
>   /* Return info from the table */
>   return nand_isbad_bbt(mtd, ofs, allowbbt);
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 3e92be1..c15818e 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -650,7 +650,7 @@ struct nand_chip {
>   void (*write_buf)(struct mtd_info *mtd, const uint8_t *buf, int len);
>   void (*read_buf)(struct mtd_info *mtd, uint8_t *buf, int len);
>   void (*select_chip)(struct mtd_info *mtd, int chip);
> - int (*block_bad)(struct mtd_info *mtd, loff_t ofs, int getchip);
> + int (*block_bad)(struct mtd_info *mtd, loff_t ofs);
>   int (*block_markbad)(struct mtd_info *mtd, loff_t ofs);
>   void (*cmd_ctrl)(struct mtd_info *mtd, int dat, unsigned int ctrl);
>   int (*dev_ready)(struct mtd_info *mtd);



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 3/3] dt/bindings: qcom_nandc: Add DT bindings

2016-01-06 Thread Boris Brezillon 
On Wed, 6 Jan 2016 09:14:44 -0600
Rob Herring  wrote:

> On Tue, Jan 05, 2016 at 10:55:01AM +0530, Archit Taneja wrote:
> > Add DT bindings document for the Qualcomm NAND controller driver.
> > 
> > Cc: devicet...@vger.kernel.org
> > Cc: Rob Herring 
> > Signed-off-by: Archit Taneja 
> > ---
> > v5:
> > - Make changes to incorporate chip select sub nodes (brcmnand taken as
> >   reference)
> > 
> > v3:
> > - Don't use '0x' when specifying nand controller address space
> > - Add optional property for on-flash bbt usage
> > 
> >  .../devicetree/bindings/mtd/qcom_nandc.txt | 84 
> > ++
> >  1 file changed, 84 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt 
> > b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> > new file mode 100644
> > index 000..b2cf2d9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> > @@ -0,0 +1,84 @@
> > +* Qualcomm NAND controller
> > +
> > +Required properties:
> > +- compatible:  should be "qcom,ebi2-nand" for IPQ806x
> 
> More specific name please.
> 
> > +- reg: MMIO address range
> > +- clocks:  must contain core clock and always on clock
> > +- clock-names: must contain "core" for the core clock and 
> > "aon" for the
> > +   always on clock
> > +- dmas:DMA specifier, consisting of a phandle to the 
> > ADM DMA
> > +   controller node and the channel number to be used for
> > +   NAND. Refer to dma.txt and qcom_adm.txt for more details
> > +- dma-names:   must be "rxtx"
> > +- qcom,cmd-crci:   must contain the ADM command type CRCI block instance
> > +   number specified for the NAND controller on the given
> > +   platform
> > +- qcom,data-crci:  must contain the ADM data type CRCI block instance
> > +   number specified for the NAND controller on the given
> > +   platform
> > +- #address-cells:  <1> - subnodes give the chip-select number
> > +- #size-cells: <0>
> > +
> > +* NAND chip-select
> > +
> > +Each controller may contain one or more subnodes to represent enabled
> > +chip-selects which (may) contain NAND flash chips. Their properties are as
> > +follows.
> > +
> > +Required properties:
> > +- compatible:  should contain "qcom,nandcs"
> > +- reg: a single integer representing the chip-select
> > +   number (e.g., 0, 1, 2, etc.)
> > +- #address-cells:  see partition.txt
> > +- #size-cells: see partition.txt
> > +- nand-ecc-strength:   number of bits to correct per ECC step. Must be 
> > 4 or 8
> > +   bits.
> > +- nand-ecc-step-size:  bytes of data per ECC step. Must be 512.
> > +
> > +Optional properties:
> > +- nand-bus-width:  bus width. Must be 8 or 16. If not present, 8 is chosen
> > +   as default
> > +
> > +Each nandcs device node may optionally contain sub-nodes describing the 
> > flash
> > +partition mapping. See partition.txt for more detail.
> 
> Can't the partitioning span across chip selects? After all, interleaving 
> is how you get high performance.

Hm, defining aggregated mtd devices in the DT is not supported, and
since, as I've been told many times ;), DT is supposed to represent the
HW not what we want to do with it, I'm not sure that's such a good idea.

Note that the infrastructure to concatenate several MTD devices
already exists, but it's not used by the NAND layer.

Also note that some NAND chips embed several dies and expose multiple
CS lines. The NAND framework already supports assigning different CS
lines to a single NAND device, so you could abuse this feature and
expose your different NAND devices as a single one (that will only work
for a cluster of identical chips connected to the same controller), but
I'd really like to avoid this kind of things.

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses

2016-01-06 Thread Bjorn Helgaas 
[+cc Jisheng]

On Fri, Dec 18, 2015 at 02:38:55PM +0200, Stanimir Varbanov wrote:
> There is no guarantees that enabling ATU will hit the hardware
> immediately, and subsequent accesses to configuration / IO spaces
> are reliable. So fixing this by read back PCIE_ATU_CR2 register
> just after writing.
> 
> Without such a fix the PCI device enumeration during kernel boot
> is not reliable, and reading configuration space for particular
> PCI device on the bus returns zero aka no device.
> 
> Signed-off-by: Stanimir Varbanov 
> ---
>  drivers/pci/host/pcie-designware.c |7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/pci/host/pcie-designware.c 
> b/drivers/pci/host/pcie-designware.c
> index 02a7452bdf23..7880de63895d 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -154,6 +154,8 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int 
> where, int size,
>  static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
>   int type, u64 cpu_addr, u64 pci_addr, u32 size)
>  {
> + u32 val;
> +
>   dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
> PCIE_ATU_VIEWPORT);
>   dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr), PCIE_ATU_LOWER_BASE);
> @@ -164,6 +166,11 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port 
> *pp, int index,
>   dw_pcie_writel_rc(pp, upper_32_bits(pci_addr), PCIE_ATU_UPPER_TARGET);
>   dw_pcie_writel_rc(pp, type, PCIE_ATU_CR1);
>   dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2);
> + /*
> +  * ensure that the ATU enable has been happaned before accessing
> +  * pci configuration/io spaces through dw_pcie_cfg_[read|write].
> +  */
> + dw_pcie_readl_rc(pp, PCIE_ATU_CR2, );

This particular fix makes sense to me.

But I have a larger question about how the ATU works.  I see these
definitions:

  #define PCIE_ATU_TYPE_MEM
  #define PCIE_ATU_TYPE_IO
  #define PCIE_ATU_TYPE_CFG0
  #define PCIE_ATU_TYPE_CFG1

and these uses:

  - In dw_pcie_host_init(), set PCIE_ATU_TYPE_MEM for unit 1
(but only if rd_other_conf is not overridden)

  - In dw_pcie_rd_other_conf() and dw_pcie_wr_other_conf(),
set PCIE_ATU_TYPE_CFG0 before config access to own bus;
set PCIE_ATU_TYPE_CFG1 before config access to other bus;
set PCIE_ATU_TYPE_IO after completion

I'm confused:

1) I assume PCIE_ATU_TYPE_MEM is for access to PCI memory space.  Why
is that initialization related to rd_other_conf?  Shouldn't that be
set up always?  A comment here would be nice, to clarify that this is
not related to the subsequent dw_pcie_wr_own_conf() calls.

2) Why doesn't dw_pcie_host_init() use dw_pcie_rd_conf() instead of
dw_pcie_rd_own_conf()?  Using pci_read_config_dword() might be even
better.  Using the internal interfaces piecemeal like we do today is
just an opportunity for doing it wrong.

3) The definitions and your comment about "accessing PCI
configuration/io spaces" suggest that the ATU must be programmed
differently for accesses to PCI config space vs. I/O space.  If that's
the case, we'd need some kind of mutex to protect inl(), etc., during
config accesses.  For example, in this scenario:

  thread A thread B
  ---
  pci_read_config_dword()
  dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_CFG0)
   inl()
  dw_pcie_cfg_read()
  readl()
  dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_IO)
   inl()

Do both inl() calls by thread B work correctly, even though the ATU
seems to be programmed for CFG0 for the first but IO for the second?

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

Re: [PATCH v5 3/3] dt/bindings: qcom_nandc: Add DT bindings

2016-01-06 Thread Rob Herring 
On Wed, Jan 6, 2016 at 9:37 AM, Boris Brezillon
 wrote:
> On Wed, 6 Jan 2016 09:14:44 -0600
> Rob Herring  wrote:
>
>> On Tue, Jan 05, 2016 at 10:55:01AM +0530, Archit Taneja wrote:
>> > Add DT bindings document for the Qualcomm NAND controller driver.
>> >
>> > Cc: devicet...@vger.kernel.org
>> > Cc: Rob Herring 
>> > Signed-off-by: Archit Taneja 
>> > ---
>> > v5:
>> > - Make changes to incorporate chip select sub nodes (brcmnand taken as
>> >   reference)
>> >
>> > v3:
>> > - Don't use '0x' when specifying nand controller address space
>> > - Add optional property for on-flash bbt usage
>> >
>> >  .../devicetree/bindings/mtd/qcom_nandc.txt | 84 
>> > ++
>> >  1 file changed, 84 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt 
>> > b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
>> > new file mode 100644
>> > index 000..b2cf2d9
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
>> > @@ -0,0 +1,84 @@
>> > +* Qualcomm NAND controller
>> > +
>> > +Required properties:
>> > +- compatible:  should be "qcom,ebi2-nand" for IPQ806x
>>
>> More specific name please.
>>
>> > +- reg: MMIO address range
>> > +- clocks:  must contain core clock and always on clock
>> > +- clock-names: must contain "core" for the core clock and 
>> > "aon" for the
>> > +   always on clock
>> > +- dmas:DMA specifier, consisting of a phandle to the 
>> > ADM DMA
>> > +   controller node and the channel number to be used for
>> > +   NAND. Refer to dma.txt and qcom_adm.txt for more 
>> > details
>> > +- dma-names:   must be "rxtx"
>> > +- qcom,cmd-crci:   must contain the ADM command type CRCI block instance
>> > +   number specified for the NAND controller on the given
>> > +   platform
>> > +- qcom,data-crci:  must contain the ADM data type CRCI block instance
>> > +   number specified for the NAND controller on the given
>> > +   platform
>> > +- #address-cells:  <1> - subnodes give the chip-select number
>> > +- #size-cells: <0>
>> > +
>> > +* NAND chip-select
>> > +
>> > +Each controller may contain one or more subnodes to represent enabled
>> > +chip-selects which (may) contain NAND flash chips. Their properties are as
>> > +follows.
>> > +
>> > +Required properties:
>> > +- compatible:  should contain "qcom,nandcs"
>> > +- reg: a single integer representing the chip-select
>> > +   number (e.g., 0, 1, 2, etc.)
>> > +- #address-cells:  see partition.txt
>> > +- #size-cells: see partition.txt
>> > +- nand-ecc-strength:   number of bits to correct per ECC step. Must 
>> > be 4 or 8
>> > +   bits.
>> > +- nand-ecc-step-size:  bytes of data per ECC step. Must be 512.
>> > +
>> > +Optional properties:
>> > +- nand-bus-width:  bus width. Must be 8 or 16. If not present, 8 is chosen
>> > +   as default
>> > +
>> > +Each nandcs device node may optionally contain sub-nodes describing the 
>> > flash
>> > +partition mapping. See partition.txt for more detail.
>>
>> Can't the partitioning span across chip selects? After all, interleaving
>> is how you get high performance.
>
> Hm, defining aggregated mtd devices in the DT is not supported, and
> since, as I've been told many times ;), DT is supposed to represent the
> HW not what we want to do with it, I'm not sure that's such a good idea.

What are partitions then?

> Note that the infrastructure to concatenate several MTD devices
> already exists, but it's not used by the NAND layer.
>
> Also note that some NAND chips embed several dies and expose multiple
> CS lines. The NAND framework already supports assigning different CS
> lines to a single NAND device, so you could abuse this feature and
> expose your different NAND devices as a single one (that will only work
> for a cluster of identical chips connected to the same controller), but
> I'd really like to avoid this kind of things.

What exactly the kernel supports ATM is irrelevant. I'm fully aware
that the kernel's NAND support has huge gaps in its ability to support
raw NAND at typical SSD speeds. My last employer had delusions about
doing that. Fortunately, NAND manufacturers don't really want to sell
you raw NAND anyway.

My point here was only that the partitions node may not be under the
CS nodes, but at the same level. Or maybe partitions in DT just
doesn't make sense at all for interleaved cases.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo

Re: [PATCH v5 2/3] mtd: nand: Qualcomm NAND controller driver

2016-01-06 Thread Boris Brezillon 
Hi Archit,

On Tue,  5 Jan 2016 10:55:00 +0530
Archit Taneja  wrote:

> The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
> MDM9x15 series.
> 
> It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
> and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
> broader interface for external slow peripheral devices such as LCD and
> NAND/NOR flash memory or SRAM like interfaces.
> 
> We add support for the NAND controller found within EBI2. For the SoCs
> of our interest, we only use the NAND controller within EBI2. Therefore,
> it's safe for us to assume that the NAND controller is a standalone block
> within the SoC.
> 
> The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
> flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
> 16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
> and spare data. The controller contains an internal 512 byte page buffer
> to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
> for register read/write and data transfers. The controller performs page
> reads and writes at a codeword/step level of 512 bytes. It can support up
> to 2 external chips of different configurations.
> 
> The driver prepares register read and write configuration descriptors for
> each codeword, followed by data descriptors to read or write data from the
> controller's internal buffer. It uses a single ADM DMA channel that we get
> via dmaengine API. The controller requires 2 ADM CRCIs for command and
> data flow control. These are passed via DT.
> 
> The ecc layout used by the controller is syndrome like, but we can't use
> the standard syndrome ecc ops because of several reasons. First, the amount
> of data bytes covered by ecc isn't same in each step. Second, writing to
> free oob space requires us writing to the entire step in which the oob
> lies. This forces us to create our own ecc ops.
> 
> One more difference is how the controller accesses the bad block marker.
> The controller ignores reading the marker when ECC is enabled. ECC needs
> to be explicity disabled to read or write to the bad block marker. The
> nand_bbt helpers library hence can't access BBMs for the controller.
> For now, we skip the creation of BBT and populate chip->block_bad and
> chip->block_markbad helpers instead.
> 
> Reviewed-by: Andy Gross 
> Reviewed-by: Stephen Boyd 
> Signed-off-by: Stephen Boyd 
> Signed-off-by: Archit Taneja 
> ---
> v5:
>   - split chip/controller structs
>   - simplify layout by considering reserved bytes as part of ECC
>   - create ecc layouts automatically
>   - implement block_bad and block_markbad chip ops instead of
>   - read_oob_raw/write_oob_raw ecc ops to access BBMs.
>   - Add NAND_SKIP_BBTSCAN flag until we get badblockbits support.
>   - misc clean ups
> 
> v4:
>   - Shrink submit_descs
>   - add desc list node at the end of dma_prep_desc
>   - Endianness and warning fixes
>   - Add Stephen's Signed-off since he provided a patch to fix
> endianness problems
> 
> v3:
>   - Refactor dma functions for maximum reuse
>   - Use dma_slave_confing on stack
>   - optimize and clean upempty_page_fixup using memchr_inv
>   - ensure portability with dma register reads using le32_* funcs
>   - use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
>   - fix handling of return values of dmaengine funcs
>   - constify wherever possible
>   - Remove dependency on ADM DMA in Kconfig
>   - Misc fixes and clean ups
> 
> v2:
>   - Use new BBT flag that allows us to read BBM in raw mode
>   - reduce memcpy-s in the driver
>   - some refactor and clean ups because of above changes
> 
>  drivers/mtd/nand/Kconfig  |7 +
>  drivers/mtd/nand/Makefile |1 +
>  drivers/mtd/nand/qcom_nandc.c | 1981 
> +
>  3 files changed, 1989 insertions(+)
>  create mode 100644 drivers/mtd/nand/qcom_nandc.c
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 95b8d2b..2fccdfb 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -546,4 +546,11 @@ config MTD_NAND_HISI504
>   help
> Enables support for NAND controller on Hisilicon SoC Hip04.
>  
> +config MTD_NAND_QCOM
> + tristate "Support for NAND on QCOM SoCs"
> + depends on ARCH_QCOM
> + help
> +   Enables support for NAND flash chips on SoCs containing the EBI2 NAND
> +   controller. This controller is found on IPQ806x SoC.
> +
>  endif # MTD_NAND
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index 2c7f014..9450cdc 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -55,5 +55,6 @@ obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)+= 
> bcm47xxnflash/
>  obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o
>

Re: [PATCH 1/3] driver-core: platform: Add platform_irq_count()

2016-01-06 Thread Rob Herring 
On Wed, Jan 6, 2016 at 7:12 PM, Stephen Boyd  wrote:
> A recent patch added calls to of_irq_count() in the qcom pinctrl
> drivers and that caused module build failures because
> of_irq_count() is not an exported symbol. We shouldn't export
> of_irq_count() to modules because it's an internal OF API that
> shouldn't be used by drivers. Platform drivers should use
> platform device APIs instead. Therefore, add a platform_irq_count()
> API that mirrors the of_irq_count() API so that platform drivers
> can stay DT agnostic.
>
> Cc: Rob Herring 
> Cc: Greg Kroah-Hartman 
> Cc: Andy Gross 
> Signed-off-by: Stephen Boyd 

Acked-by: Rob Herring 
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] driver-core: platform: Add platform_irq_count()

2016-01-06 Thread Greg Kroah-Hartman 
On Wed, Jan 06, 2016 at 05:12:47PM -0800, Stephen Boyd wrote:
> A recent patch added calls to of_irq_count() in the qcom pinctrl
> drivers and that caused module build failures because
> of_irq_count() is not an exported symbol. We shouldn't export
> of_irq_count() to modules because it's an internal OF API that
> shouldn't be used by drivers. Platform drivers should use
> platform device APIs instead. Therefore, add a platform_irq_count()
> API that mirrors the of_irq_count() API so that platform drivers
> can stay DT agnostic.
> 
> Cc: Rob Herring 
> Cc: Greg Kroah-Hartman 
> Cc: Andy Gross 
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/base/platform.c | 20 
>  include/linux/platform_device.h |  1 +
>  2 files changed, 21 insertions(+)

Acked-by: Greg Kroah-Hartman

Re: [PATCH] mfd: qcom-spmi-pmic: Don't access non-existing registers

2016-01-06 Thread Stephen Boyd 
On 11/17/15 16:06, Stephen Boyd wrote:
> From: "Ivan T. Ivanov" 
>
> Revision ID registers are available only on devices with
> Slave IDs that are even, so don't make access to unavailable
> registers.
>
> Signed-off-by: Ivan T. Ivanov 
> [sb...@codeaurora.org: Consider all slave ids that are even]
> Signed-off-by: Stephen Boyd 
> ---

Ping?

>  drivers/mfd/qcom-spmi-pmic.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/qcom-spmi-pmic.c b/drivers/mfd/qcom-spmi-pmic.c
> index af6ac1c4b45c..8653e8b9bb4f 100644
> --- a/drivers/mfd/qcom-spmi-pmic.c
> +++ b/drivers/mfd/qcom-spmi-pmic.c
> @@ -127,7 +127,9 @@ static int pmic_spmi_probe(struct spmi_device *sdev)
>   if (IS_ERR(regmap))
>   return PTR_ERR(regmap);
>  
> - pmic_spmi_show_revid(regmap, >dev);
> + /* Only the first slave id for a PMIC contains this information */
> + if (sdev->usid % 2 == 0)
> + pmic_spmi_show_revid(regmap, >dev);
>  
>   return of_platform_populate(root, NULL, NULL, >dev);
>  }

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

Re: [PATCH v5 1/3] mtd: nand: don't select chip in nand_chip's block_bad op

2016-01-06 Thread Archit Taneja 



On 01/06/2016 09:35 PM, Boris Brezillon wrote:

On Tue,  5 Jan 2016 10:54:59 +0530
Archit Taneja  wrote:


One of the arguments passed to struct nand_chip's block_bad op is
'getchip', which, if true, is supposed to get and select the nand device,
and later unselect and release the device.

This op is intended to be replaceable by drivers. The drivers shouldn't
be responsible for selecting/unselecting chip. Like other ops, the chip
should already be selected before the block_bad op is called.

Remove the getchip argument from the block_bad op and move the chip
selection to nand_block_checkbad (the only user of chip->block_bad).
Modify nand_block_bad (the default function for the op) such that
it doesn't select the chip.


I would go even further and move the chip selection into
nand_block_isbad(), who is the only caller that requires this chip
selection.


You're right. The only other place where nand_block_checkbad is used is
nand_erase_nand, and that already selects the chip before.

I'll update the patch.

Thanks,
Archit





Signed-off-by: Archit Taneja 
---
  drivers/mtd/nand/nand_base.c | 41 +++--
  include/linux/mtd/nand.h |  2 +-
  2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 928081b..42aa441 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -317,9 +317,9 @@ static void nand_read_buf16(struct mtd_info *mtd, uint8_t 
*buf, int len)
   *
   * Check, if the block is bad.
   */
-static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
+static int nand_block_bad(struct mtd_info *mtd, loff_t ofs)
  {
-   int page, chipnr, res = 0, i = 0;
+   int page, res = 0, i = 0;
struct nand_chip *chip = mtd_to_nand(mtd);
u16 bad;

@@ -328,15 +328,6 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t 
ofs, int getchip)

page = (int)(ofs >> chip->page_shift) & chip->pagemask;

-   if (getchip) {
-   chipnr = (int)(ofs >> chip->chip_shift);
-
-   nand_get_device(mtd, FL_READING);
-
-   /* Select the NAND device */
-   chip->select_chip(mtd, chipnr);
-   }
-
do {
if (chip->options & NAND_BUSWIDTH_16) {
chip->cmdfunc(mtd, NAND_CMD_READOOB,
@@ -361,11 +352,6 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t 
ofs, int getchip)
i++;
} while (!res && i < 2 && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE));

-   if (getchip) {
-   chip->select_chip(mtd, -1);
-   nand_release_device(mtd);
-   }
-
return res;
  }

@@ -514,8 +500,27 @@ static int nand_block_checkbad(struct mtd_info *mtd, 
loff_t ofs, int getchip,
  {
struct nand_chip *chip = mtd_to_nand(mtd);

-   if (!chip->bbt)
-   return chip->block_bad(mtd, ofs, getchip);
+   if (!chip->bbt) {
+   int ret;
+
+   if (getchip) {
+   int chipnr = (int)(ofs >> chip->chip_shift);
+
+   nand_get_device(mtd, FL_READING);
+
+   /* Select the NAND device */
+   chip->select_chip(mtd, chipnr);
+   }
+
+   ret = chip->block_bad(mtd, ofs);
+
+   if (getchip) {
+   chip->select_chip(mtd, -1);
+   nand_release_device(mtd);
+   }
+
+   return ret;
+   }

/* Return info from the table */
return nand_isbad_bbt(mtd, ofs, allowbbt);
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 3e92be1..c15818e 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -650,7 +650,7 @@ struct nand_chip {
void (*write_buf)(struct mtd_info *mtd, const uint8_t *buf, int len);
void (*read_buf)(struct mtd_info *mtd, uint8_t *buf, int len);
void (*select_chip)(struct mtd_info *mtd, int chip);
-   int (*block_bad)(struct mtd_info *mtd, loff_t ofs, int getchip);
+   int (*block_bad)(struct mtd_info *mtd, loff_t ofs);
int (*block_markbad)(struct mtd_info *mtd, loff_t ofs);
void (*cmd_ctrl)(struct mtd_info *mtd, int dat, unsigned int ctrl);
int (*dev_ready)(struct mtd_info *mtd);






--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum, hosted by The Linux Foundation

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

Re: [PATCH 2/3] pinctrl: qcom: Use platform_irq_count() instead of of_irq_count()

2016-01-06 Thread Stephen Boyd 
On 01/06/16 17:19, Bjorn Andersson wrote:
> On Wed, Jan 6, 2016 at 5:12 PM, Stephen Boyd  wrote:
>> of_irq_count() is not an exported symbol (and it shouldn't be
>> used by platform drivers anyway) so use platform_irq_count()
>> instead. This allows us to make the qcom pinctrl drivers modular
>> again.
>>
> [..]
>> diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c 
>> b/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
>> index 3ddb4cc38f1c..37ae6b72ea35 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
>> @@ -758,16 +758,19 @@ static int pm8xxx_mpp_probe(struct platform_device 
>> *pdev)
>> struct pinctrl_pin_desc *pins;
>> struct pm8xxx_mpp *pctrl;
>> int ret;
>> -   int i;
>> +   int i, npins;
>>
>> pctrl = devm_kzalloc(>dev, sizeof(*pctrl), GFP_KERNEL);
>> if (!pctrl)
>> return -ENOMEM;
>>
>> pctrl->dev = >dev;
>> -   pctrl->npins = of_irq_count(pdev->dev.of_node);
>> -   if (!pctrl->npins)
>> +   npins = of_irq_count(pdev->dev.of_node);
> platform_irq_count(pdev)

Ouch. So many duplicates the odds were against me.


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[PATCH v2 2/3] pinctrl: qcom: Use platform_irq_count() instead of of_irq_count()

2016-01-06 Thread Stephen Boyd 
of_irq_count() is not an exported symbol (and it shouldn't be
used by platform drivers anyway) so use platform_irq_count()
instead. This allows us to make the qcom pinctrl drivers modular
again.

Cc: Rob Herring 
Cc: Andy Gross 
Cc: Bjorn Andersson 
Signed-off-by: Stephen Boyd 
---

Changes from v1:
 * Fixed one wrong of_irq_count() noticed by Bjorn

 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 4 +++-
 drivers/pinctrl/qcom/pinctrl-spmi-mpp.c  | 4 +++-
 drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c | 9 ++---
 drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c  | 9 ++---
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c 
b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index f46dbbf7ff25..4e12ded3c773 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -697,9 +697,11 @@ static int pmic_gpio_probe(struct platform_device *pdev)
return ret;
}
 
-   npins = of_irq_count(dev->of_node);
+   npins = platform_irq_count(pdev);
if (!npins)
return -EINVAL;
+   if (npins < 0)
+   return npins;
 
BUG_ON(npins > ARRAY_SIZE(pmic_gpio_groups));
 
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c 
b/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
index 5c7935012e3a..2f18323571a6 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
@@ -799,9 +799,11 @@ static int pmic_mpp_probe(struct platform_device *pdev)
return ret;
}
 
-   npins = of_irq_count(dev->of_node);
+   npins = platform_irq_count(pdev);
if (!npins)
return -EINVAL;
+   if (npins < 0)
+   return npins;
 
BUG_ON(npins > ARRAY_SIZE(pmic_mpp_groups));
 
diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c 
b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
index f75e482073b7..cd8580d9741d 100644
--- a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
@@ -667,16 +667,19 @@ static int pm8xxx_gpio_probe(struct platform_device *pdev)
struct pinctrl_pin_desc *pins;
struct pm8xxx_gpio *pctrl;
int ret;
-   int i;
+   int i, npins;
 
pctrl = devm_kzalloc(>dev, sizeof(*pctrl), GFP_KERNEL);
if (!pctrl)
return -ENOMEM;
 
pctrl->dev = >dev;
-   pctrl->npins = of_irq_count(pdev->dev.of_node);
-   if (!pctrl->npins)
+   npins = platform_irq_count(pdev);
+   if (!npins)
return -EINVAL;
+   if (npins < 0)
+   return npins;
+   pctrl->npins = npins;
 
pctrl->regmap = dev_get_regmap(pdev->dev.parent, NULL);
if (!pctrl->regmap) {
diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c 
b/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
index 3e19c12a315a..54a5402a9079 100644
--- a/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
+++ b/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
@@ -758,16 +758,19 @@ static int pm8xxx_mpp_probe(struct platform_device *pdev)
struct pinctrl_pin_desc *pins;
struct pm8xxx_mpp *pctrl;
int ret;
-   int i;
+   int i, npins;
 
pctrl = devm_kzalloc(>dev, sizeof(*pctrl), GFP_KERNEL);
if (!pctrl)
return -ENOMEM;
 
pctrl->dev = >dev;
-   pctrl->npins = of_irq_count(pdev->dev.of_node);
-   if (!pctrl->npins)
+   npins = platform_irq_count(pdev);
+   if (!npins)
return -EINVAL;
+   if (npins < 0)
+   return npins;
+   pctrl->npins = npins;
 
pctrl->regmap = dev_get_regmap(pdev->dev.parent, NULL);
if (!pctrl->regmap) {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

Re: [PATCH v2 1/5] devicetree: bindings: Document qcom board compatible format

2016-01-06 Thread Stephen Boyd 
On 11/23/15 16:47, Stephen Boyd wrote:
> On 11/22, Rob Herring wrote:
>>
>> Much more reasonable now. I do find the '/' in it a bit strange though.
> I can remove the backslash if you like. Is a dash more preferred?
>
>> Acked-by: Rob Herring 
>>
> and if so can I keep the ack? I'll resend with that change and
> add the ack.
>

I'll take no answer as "let's not change anything", so can someone
pickup this patch as is? Perhaps Andy can do that?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[PATCH 1/3] driver-core: platform: Add platform_irq_count()

2016-01-06 Thread Stephen Boyd 
A recent patch added calls to of_irq_count() in the qcom pinctrl
drivers and that caused module build failures because
of_irq_count() is not an exported symbol. We shouldn't export
of_irq_count() to modules because it's an internal OF API that
shouldn't be used by drivers. Platform drivers should use
platform device APIs instead. Therefore, add a platform_irq_count()
API that mirrors the of_irq_count() API so that platform drivers
can stay DT agnostic.

Cc: Rob Herring 
Cc: Greg Kroah-Hartman 
Cc: Andy Gross 
Signed-off-by: Stephen Boyd 
---
 drivers/base/platform.c | 20 
 include/linux/platform_device.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index d77ed0c946dd..8dcbb266643b 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -118,6 +118,26 @@ int platform_get_irq(struct platform_device *dev, unsigned 
int num)
 EXPORT_SYMBOL_GPL(platform_get_irq);
 
 /**
+ * platform_irq_count - Count the number of IRQs a platform device uses
+ * @dev: platform device
+ *
+ * Return: Number of IRQs a platform device uses or EPROBE_DEFER
+ */
+int platform_irq_count(struct platform_device *dev)
+{
+   int ret, nr = 0;
+
+   while ((ret = platform_get_irq(dev, nr)) >= 0)
+   nr++;
+
+   if (ret == -EPROBE_DEFER)
+   return ret;
+
+   return nr;
+}
+EXPORT_SYMBOL_GPL(platform_irq_count);
+
+/**
  * platform_get_resource_byname - get a resource for a device by name
  * @dev: platform device
  * @type: resource type
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index dba40b1c41dc..03b755521fd9 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -52,6 +52,7 @@ extern void arch_setup_pdev_archdata(struct platform_device 
*);
 extern struct resource *platform_get_resource(struct platform_device *,
  unsigned int, unsigned int);
 extern int platform_get_irq(struct platform_device *, unsigned int);
+extern int platform_irq_count(struct platform_device *);
 extern struct resource *platform_get_resource_byname(struct platform_device *,
 unsigned int,
 const char *);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[PATCH 2/3] pinctrl: qcom: Use platform_irq_count() instead of of_irq_count()

2016-01-06 Thread Stephen Boyd 
of_irq_count() is not an exported symbol (and it shouldn't be
used by platform drivers anyway) so use platform_irq_count()
instead. This allows us to make the qcom pinctrl drivers modular
again.

Cc: Rob Herring 
Cc: Andy Gross 
Signed-off-by: Stephen Boyd 
---
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 4 +++-
 drivers/pinctrl/qcom/pinctrl-spmi-mpp.c  | 4 +++-
 drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c | 9 ++---
 drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c  | 9 ++---
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c 
b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index 60b922274879..9ed2b4eba1f8 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -702,9 +702,11 @@ static int pmic_gpio_probe(struct platform_device *pdev)
return ret;
}
 
-   npins = of_irq_count(dev->of_node);
+   npins = platform_irq_count(pdev);
if (!npins)
return -EINVAL;
+   if (npins < 0)
+   return npins;
 
BUG_ON(npins > ARRAY_SIZE(pmic_gpio_groups));
 
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c 
b/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
index 5654287c382c..3fd04a6888a4 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
@@ -804,9 +804,11 @@ static int pmic_mpp_probe(struct platform_device *pdev)
return ret;
}
 
-   npins = of_irq_count(dev->of_node);
+   npins = platform_irq_count(pdev);
if (!npins)
return -EINVAL;
+   if (npins < 0)
+   return npins;
 
BUG_ON(npins > ARRAY_SIZE(pmic_mpp_groups));
 
diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c 
b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
index df054e716dc0..bde2aa684dc1 100644
--- a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
@@ -667,16 +667,19 @@ static int pm8xxx_gpio_probe(struct platform_device *pdev)
struct pinctrl_pin_desc *pins;
struct pm8xxx_gpio *pctrl;
int ret;
-   int i;
+   int i, npins;
 
pctrl = devm_kzalloc(>dev, sizeof(*pctrl), GFP_KERNEL);
if (!pctrl)
return -ENOMEM;
 
pctrl->dev = >dev;
-   pctrl->npins = of_irq_count(pdev->dev.of_node);
-   if (!pctrl->npins)
+   npins = platform_irq_count(pdev);
+   if (!npins)
return -EINVAL;
+   if (npins < 0)
+   return npins;
+   pctrl->npins = npins;
 
pctrl->regmap = dev_get_regmap(pdev->dev.parent, NULL);
if (!pctrl->regmap) {
diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c 
b/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
index 3ddb4cc38f1c..37ae6b72ea35 100644
--- a/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
+++ b/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
@@ -758,16 +758,19 @@ static int pm8xxx_mpp_probe(struct platform_device *pdev)
struct pinctrl_pin_desc *pins;
struct pm8xxx_mpp *pctrl;
int ret;
-   int i;
+   int i, npins;
 
pctrl = devm_kzalloc(>dev, sizeof(*pctrl), GFP_KERNEL);
if (!pctrl)
return -ENOMEM;
 
pctrl->dev = >dev;
-   pctrl->npins = of_irq_count(pdev->dev.of_node);
-   if (!pctrl->npins)
+   npins = of_irq_count(pdev->dev.of_node);
+   if (!npins)
return -EINVAL;
+   if (npins < 0)
+   return npins;
+   pctrl->npins = npins;
 
pctrl->regmap = dev_get_regmap(pdev->dev.parent, NULL);
if (!pctrl->regmap) {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

Re: [PATCH 2/3] pinctrl: qcom: Use platform_irq_count() instead of of_irq_count()

2016-01-06 Thread Bjorn Andersson 
On Wed, Jan 6, 2016 at 5:12 PM, Stephen Boyd  wrote:
> of_irq_count() is not an exported symbol (and it shouldn't be
> used by platform drivers anyway) so use platform_irq_count()
> instead. This allows us to make the qcom pinctrl drivers modular
> again.
>
[..]
> diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c 
> b/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
> index 3ddb4cc38f1c..37ae6b72ea35 100644
> --- a/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
> +++ b/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
> @@ -758,16 +758,19 @@ static int pm8xxx_mpp_probe(struct platform_device 
> *pdev)
> struct pinctrl_pin_desc *pins;
> struct pm8xxx_mpp *pctrl;
> int ret;
> -   int i;
> +   int i, npins;
>
> pctrl = devm_kzalloc(>dev, sizeof(*pctrl), GFP_KERNEL);
> if (!pctrl)
> return -ENOMEM;
>
> pctrl->dev = >dev;
> -   pctrl->npins = of_irq_count(pdev->dev.of_node);
> -   if (!pctrl->npins)
> +   npins = of_irq_count(pdev->dev.of_node);

platform_irq_count(pdev)

> +   if (!npins)
> return -EINVAL;
> +   if (npins < 0)
> +   return npins;
> +   pctrl->npins = npins;
>
> pctrl->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> if (!pctrl->regmap) {

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] arm: dts: qcom: Add more board clocks

2016-01-06 Thread Stephen Boyd 
These clocks are fixed rate board sources that should be in DT.
Add them.

Cc: Georgi Djakov 
Signed-off-by: Stephen Boyd 
---

It seems that I never sent out the updated version of this patch
to cover all the SoCs we have. This is on top of linux-next.

 arch/arm/boot/dts/qcom-apq8084.dtsi | 14 ++
 arch/arm/boot/dts/qcom-ipq8064.dtsi | 12 
 arch/arm/boot/dts/qcom-msm8660.dtsi | 20 
 arch/arm/boot/dts/qcom-msm8974.dtsi | 14 ++
 4 files changed, 60 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi 
b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 08214cbae16d..a33a09f6821e 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -91,6 +91,20 @@
interrupts = <1 7 0xf04>;
};
 
+   clocks {
+   xo_board {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <1920>;
+   };
+
+   sleep_clk {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <32768>;
+   };
+   };
+
timer {
compatible = "arm,armv7-timer";
interrupts = <1 2 0xf08>,
diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi 
b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index fa698635eea0..2601a907947b 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -62,6 +62,18 @@
};
 
clocks {
+   cxo_board {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <1920>;
+   };
+
+   pxo_board {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <2700>;
+   };
+
sleep_clk: sleep_clk {
compatible = "fixed-clock";
clock-frequency = <32768>;
diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi 
b/arch/arm/boot/dts/qcom-msm8660.dtsi
index e5f7f33aa467..a4b184db21d0 100644
--- a/arch/arm/boot/dts/qcom-msm8660.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8660.dtsi
@@ -42,6 +42,26 @@
interrupts = <1 9 0x304>;
};
 
+   clocks {
+   cxo_board {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <1920>;
+   };
+
+   pxo_board {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <2700>;
+   };
+
+   sleep_clk {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <32768>;
+   };
+   };
+
soc: soc {
#address-cells = <1>;
#size-cells = <1>;
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi 
b/arch/arm/boot/dts/qcom-msm8974.dtsi
index c72cbdca3d8e..c65d0d90eed3 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -137,6 +137,20 @@
interrupts = <1 7 0xf04>;
};
 
+   clocks {
+   xo_board {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <1920>;
+   };
+
+   sleep_clk {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <32768>;
+   };
+   };
+
timer {
compatible = "arm,armv7-timer";
interrupts = <1 2 0xf08>,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

Re: [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses

2016-01-06 Thread Jisheng Zhang 
Dear Bjorn,

On Wed, 6 Jan 2016 12:20:03 -0600 Bjorn Helgaas wrote:

> [+cc Jisheng]
> 
> On Fri, Dec 18, 2015 at 02:38:55PM +0200, Stanimir Varbanov wrote:
> > There is no guarantees that enabling ATU will hit the hardware
> > immediately, and subsequent accesses to configuration / IO spaces
> > are reliable. So fixing this by read back PCIE_ATU_CR2 register
> > just after writing.
> > 
> > Without such a fix the PCI device enumeration during kernel boot
> > is not reliable, and reading configuration space for particular
> > PCI device on the bus returns zero aka no device.
> > 
> > Signed-off-by: Stanimir Varbanov 
> > ---
> >  drivers/pci/host/pcie-designware.c |7 +++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/pci/host/pcie-designware.c 
> > b/drivers/pci/host/pcie-designware.c
> > index 02a7452bdf23..7880de63895d 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -154,6 +154,8 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, 
> > int where, int size,
> >  static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
> > int type, u64 cpu_addr, u64 pci_addr, u32 size)
> >  {
> > +   u32 val;
> > +
> > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
> >   PCIE_ATU_VIEWPORT);
> > dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr), PCIE_ATU_LOWER_BASE);
> > @@ -164,6 +166,11 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port 
> > *pp, int index,
> > dw_pcie_writel_rc(pp, upper_32_bits(pci_addr), PCIE_ATU_UPPER_TARGET);
> > dw_pcie_writel_rc(pp, type, PCIE_ATU_CR1);
> > dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2);
> > +   /*
> > +* ensure that the ATU enable has been happaned before accessing
> > +* pci configuration/io spaces through dw_pcie_cfg_[read|write].
> > +*/
> > +   dw_pcie_readl_rc(pp, PCIE_ATU_CR2, );  
> 
> This particular fix makes sense to me.
> 
> But I have a larger question about how the ATU works.  I see these
> definitions:
> 
>   #define PCIE_ATU_TYPE_MEM
>   #define PCIE_ATU_TYPE_IO
>   #define PCIE_ATU_TYPE_CFG0
>   #define PCIE_ATU_TYPE_CFG1
> 
> and these uses:
> 
>   - In dw_pcie_host_init(), set PCIE_ATU_TYPE_MEM for unit 1
> (but only if rd_other_conf is not overridden)
> 
>   - In dw_pcie_rd_other_conf() and dw_pcie_wr_other_conf(),
> set PCIE_ATU_TYPE_CFG0 before config access to own bus;
> set PCIE_ATU_TYPE_CFG1 before config access to other bus;
> set PCIE_ATU_TYPE_IO after completion
> 
> I'm confused:
> 
> 1) I assume PCIE_ATU_TYPE_MEM is for access to PCI memory space.  Why
> is that initialization related to rd_other_conf?  Shouldn't that be
> set up always?  A comment here would be nice, to clarify that this is
> not related to the subsequent dw_pcie_wr_own_conf() calls.

Indeed, the comment is necessary. I forget the reason until read the code
carefully again. The reason here is

If the platform provides ->rd_other_conf, it means the platform
doesn't support ATU, it uses its own address translation component
rather than ATU, so we should ignore ATU programming for this
kind of platform.

I have sent out one patch to add this comment.

> 
> 2) Why doesn't dw_pcie_host_init() use dw_pcie_rd_conf() instead of
> dw_pcie_rd_own_conf()?  Using pci_read_config_dword() might be even
> better.  Using the internal interfaces piecemeal like we do today is
> just an opportunity for doing it wrong.

IMHO, the reason is during host_init, the pci bus etc. are not initialized,
from another side, the code is really accessing its own conf registers.

> 
> 3) The definitions and your comment about "accessing PCI
> configuration/io spaces" suggest that the ATU must be programmed
> differently for accesses to PCI config space vs. I/O space.  If that's

Yes.

> the case, we'd need some kind of mutex to protect inl(), etc., during
> config accesses.  For example, in this scenario:
> 
>   thread A thread B
>   ---
>   pci_read_config_dword()
>   dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_CFG0)
>inl()
>   dw_pcie_cfg_read()
>   readl()
>   dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_IO)
>inl()
> 
> Do both inl() calls by thread B work correctly, even though the ATU
> seems to be programmed for CFG0 for the first but IO for the second?
> 

Indeed, there's such race issue as you and RMK pointed out.

IMHO, we need support:

1. platforms which contain three or more iATUs, there's no need to mux
iATU usage: one ATU for IO, one for cfg and another for MEM. But how to
get the number of iATUs, DT? eg. "snps,atu_num = 3"

2. platforms which contain only two iATUs, add mechanism to protect
the cfg vs IO race. Could you please give suggestions to how to achieve
this goal?

Thanks,
Jisheng

Re: [PATCH 1/5] [v2] usb: host: ehci-msm: Allow LS devices to work

2016-01-06 Thread Timur Tabi 

Timur Tabi wrote:

From: Jack Pham 

Disable the silicon quirk which is normally enabled for HSIC
host mode. This would otherwise prevent low speed devices
from enumerating properly.

Signed-off-by: Jack Pham 
Signed-off-by: Timur Tabi 


Felipe, any change that these five patches can make it into 4.5?

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