Hi Gilad,

Thanks for the patch.

On 01/31/2015 02:46 AM, Gilad Avidov wrote:
> Qualcomm PMIC Arbiter version-2 changes from version-1 are:
> 
> - Some different register offsets.
> - New channel register space, one per PMIC peripheral (ppid).
>   All tx traffic uses these channels.
> - New observer register space. All rx trafic uses this space.
> - Different command format for spmi command registers.
> 
> Signed-off-by: Gilad Avidov <gavi...@codeaurora.org>
> Acked-by: Sagar Dharia <sdha...@codeaurora.org>
> ---
>  .../bindings/spmi/qcom,spmi-pmic-arb.txt           |   6 +-
>  drivers/spmi/spmi-pmic-arb.c                       | 310 
> +++++++++++++++++----
>  2 files changed, 260 insertions(+), 56 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt 
> b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
> index 715d099..e16b9b5 100644
> --- a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
> +++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
> @@ -1,6 +1,6 @@
>  Qualcomm SPMI Controller (PMIC Arbiter)
>  
> -The SPMI PMIC Arbiter is found on the Snapdragon 800 Series.  It is an SPMI
> +The SPMI PMIC Arbiter is found on Snapdragon chipsets.  It is an SPMI
>  controller with wrapping arbitration logic to allow for multiple on-chip
>  devices to control a single SPMI master.
>  
> @@ -19,6 +19,10 @@ Required properties:
>       "core" - core registers
>       "intr" - interrupt controller registers
>       "cnfg" - configuration registers
> +   Registers used only for V2 PMIC Arbiter:
> +     "chnls"  - tx-channel per virtual slave registers.
> +     "obsrvr" - rx-channel (called observer) per virtual slave registers.
> +
>  - reg : address + size pairs describing the PMIC arb register sets; order 
> must
>          correspond with the order of entries in reg-names
>  - #address-cells : must be set to 2
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index 20559ab..818b2cf 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
> +/* Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.

run checkpatch, there are tons of errors like white spaces and DOS line
ending.

>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 and
> @@ -25,22 +25,18 @@
>  
>  /* PMIC Arbiter configuration registers */
>  #define PMIC_ARB_VERSION             0x0000
> +#define PMIC_ARB_VERSION_V2_MIN              (0x20010000)
>  #define PMIC_ARB_INT_EN                      0x0004
>  
> -/* PMIC Arbiter channel registers */
> -#define PMIC_ARB_CMD(N)                      (0x0800 + (0x80 * (N)))
> -#define PMIC_ARB_CONFIG(N)           (0x0804 + (0x80 * (N)))
> -#define PMIC_ARB_STATUS(N)           (0x0808 + (0x80 * (N)))
> -#define PMIC_ARB_WDATA0(N)           (0x0810 + (0x80 * (N)))
> -#define PMIC_ARB_WDATA1(N)           (0x0814 + (0x80 * (N)))
> -#define PMIC_ARB_RDATA0(N)           (0x0818 + (0x80 * (N)))
> -#define PMIC_ARB_RDATA1(N)           (0x081C + (0x80 * (N)))
> -
> -/* Interrupt Controller */
> -#define SPMI_PIC_OWNER_ACC_STATUS(M, N)      (0x0000 + ((32 * (M)) + (4 * 
> (N))))
> -#define SPMI_PIC_ACC_ENABLE(N)               (0x0200 + (4 * (N)))
> -#define SPMI_PIC_IRQ_STATUS(N)               (0x0600 + (4 * (N)))
> -#define SPMI_PIC_IRQ_CLEAR(N)                (0x0A00 + (4 * (N)))
> +/* PMIC Arbiter channel registers offsets */
> +#define PMIC_ARB_CMD                 (0x00)

no need braces here and below

> +#define PMIC_ARB_CONFIG                      (0x04)
> +#define PMIC_ARB_STATUS                      (0x08)
> +#define PMIC_ARB_WDATA0                      (0x10)
> +#define PMIC_ARB_WDATA1                      (0x14)
> +#define PMIC_ARB_RDATA0                      (0x18)
> +#define PMIC_ARB_RDATA1                      (0x1C)
> +#define PMIC_ARB_REG_CHNL(N)         (0x800 + 0x4 * (N))
>  
>  /* Mapping Table */
>  #define SPMI_MAPPING_TABLE_REG(N)    (0x0B00 + (4 * (N)))
> @@ -52,6 +48,7 @@
>  
>  #define SPMI_MAPPING_TABLE_LEN               255
>  #define SPMI_MAPPING_TABLE_TREE_DEPTH        16      /* Maximum of 16-bits */
> +#define PPID_TO_CHAN_TABLE_SZ                BIT(12) /* PPID is 12bit chan 
> is 1byte*/
>  
>  /* Ownership Table */
>  #define SPMI_OWNERSHIP_TABLE_REG(N)  (0x0700 + (4 * (N)))
> @@ -88,6 +85,7 @@ enum pmic_arb_cmd_op_code {
>  
>  /* Maximum number of support PMIC peripherals */
>  #define PMIC_ARB_MAX_PERIPHS         256
> +#define PMIC_ARB_MAX_CHNL            128
>  #define PMIC_ARB_PERIPH_ID_VALID     (1 << 15)
>  #define PMIC_ARB_TIMEOUT_US          100
>  #define PMIC_ARB_MAX_TRANS_BYTES     (8)
> @@ -98,14 +96,17 @@ enum pmic_arb_cmd_op_code {
>  /* interrupt enable bit */
>  #define SPMI_PIC_ACC_ENABLE_BIT              BIT(0)
>  
> +struct pmic_arb_ver_ops;
> +
>  /**
>   * spmi_pmic_arb_dev - SPMI PMIC Arbiter object
>   *
> - * @base:            address of the PMIC Arbiter core registers.
> + * @rd_base:         on v1 "core", on v2 "observer" register base off DT.
> + * @wr_base:         on v1 "core", on v2 "chnls"    register base off DT.
>   * @intr:            address of the SPMI interrupt control registers.
>   * @cnfg:            address of the PMIC Arbiter configuration registers.
>   * @lock:            lock to synchronize accesses.
> - * @channel:         which channel to use for accesses.
> + * @channel:         which ee channel to use for accesses.
>   * @irq:             PMIC ARB interrupt.
>   * @ee:                      the current Execution Environment
>   * @min_apid:                minimum APID (used for bounding IRQ search)
> @@ -113,10 +114,14 @@ enum pmic_arb_cmd_op_code {
>   * @mapping_table:   in-memory copy of PPID -> APID mapping table.
>   * @domain:          irq domain object for PMIC IRQ domain
>   * @spmic:           SPMI controller object
> - * @apid_to_ppid:    cached mapping from APID to PPID
> + * @apid_to_ppid:    in-memory copy of APID -> PPID mapping table.
> + * @ver_ops:         version dependent operations.
> + * @ppid_to_chan     in-memory copy of PPID -> channel (APID) mapping table.
> + *                   v2 only.
>   */
>  struct spmi_pmic_arb_dev {
> -     void __iomem            *base;
> +     void __iomem            *rd_base;
> +     void __iomem            *wr_base;
>       void __iomem            *intr;
>       void __iomem            *cnfg;
>       raw_spinlock_t          lock;
> @@ -129,17 +134,54 @@ struct spmi_pmic_arb_dev {
>       struct irq_domain       *domain;
>       struct spmi_controller  *spmic;
>       u16                     apid_to_ppid[256];
> +     const struct pmic_arb_ver_ops *ver_ops;
> +     u8                      *ppid_to_chan;
> +};
> +
> +/**
> + * pmic_arb_ver: version dependent functionality.
> + *
> + * @non_data_cmd:    on v1 issues an spmi non-data command.
> + *                   on v2 no HW support, returns -EOPNOTSUPP.
> + * @offset:          on v1 offset of per-ee channel.
> + *                   on v2 offset of per-ee and per-ppid channel.
> + * @fmt_cmd:         formats a GENI/SPMI command.
> + * @owner_acc_status:        on v1 offset of 
> PMIC_ARB_SPMI_PIC_OWNERm_ACC_STATUSn
> + *                   on v2 offset of SPMI_PIC_OWNERm_ACC_STATUSn.
> + * @acc_enable:              on v1 offset of PMIC_ARB_SPMI_PIC_ACC_ENABLEn
> + *                   on v2 offset of SPMI_PIC_ACC_ENABLEn.
> + * @irq_status:              on v1 offset of PMIC_ARB_SPMI_PIC_IRQ_STATUSn
> + *                   on v2 offset of SPMI_PIC_IRQ_STATUSn.
> + * @irq_clear:               on v1 offset of PMIC_ARB_SPMI_PIC_IRQ_CLEARn
> + *                   on v2 offset of SPMI_PIC_IRQ_CLEARn.
> + */
> +struct pmic_arb_ver_ops {
> +     /* spmi commands (read_cmd, write_cmd, cmd) functionality */
> +     u32 (*offset)(struct spmi_pmic_arb_dev *dev, u8 sid, u16 addr);
> +     u32 (*fmt_cmd)(u8 opc, u8 sid, u16 addr, u8 bc);
> +     int (*non_data_cmd)(struct spmi_controller *ctrl, u8 opc, u8 sid);
> +     /* Interrupts controller functionality (offset of PIC registers) */
> +     u32 (*owner_acc_status)(u8 m, u8 n);
> +     u32 (*acc_enable)(u8 n);
> +     u32 (*irq_status)(u8 n);
> +     u32 (*irq_clear)(u8 n);
>  };
>  

<snip>

> -static int pmic_arb_wait_for_done(struct spmi_controller *ctrl)
> +static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
> +                                     void __iomem *base, u8 sid, u16 addr)
>  {
>       struct spmi_pmic_arb_dev *dev = spmi_controller_get_drvdata(ctrl);
>       u32 status = 0;
>       u32 timeout = PMIC_ARB_TIMEOUT_US;
> -     u32 offset = PMIC_ARB_STATUS(dev->channel);
> +     u32 offset = dev->ver_ops->offset(dev, sid, addr) + PMIC_ARB_STATUS;

I'd rename this to status, it is the arbiter status. But as this is the
name in the original code it depends on you.

>  
>       while (timeout--) {
>               status = pmic_arb_base_read(dev, offset);
> @@ -211,28 +254,46 @@ static int pmic_arb_wait_for_done(struct 
> spmi_controller *ctrl)
>       return -ETIMEDOUT;
>  }
>  
> -/* Non-data command */
> -static int pmic_arb_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid)
> +static int
> +pmic_arb_non_data_cmd_v1(struct spmi_controller *ctrl, u8 opc, u8 sid)
>  {
>       struct spmi_pmic_arb_dev *pmic_arb = spmi_controller_get_drvdata(ctrl);
>       unsigned long flags;
>       u32 cmd;
>       int rc;
> -
> -     /* Check for valid non-data command */
> -     if (opc < SPMI_CMD_RESET || opc > SPMI_CMD_WAKEUP)
> -             return -EINVAL;
> +     u32 offset = pmic_arb->ver_ops->offset(pmic_arb, sid, 0);
>  
>       cmd = ((opc | 0x40) << 27) | ((sid & 0xf) << 20);
>  
>       raw_spin_lock_irqsave(&pmic_arb->lock, flags);
> -     pmic_arb_base_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
> -     rc = pmic_arb_wait_for_done(ctrl);
> +     pmic_arb_base_write(pmic_arb, offset + PMIC_ARB_CMD, cmd);
> +     rc = pmic_arb_wait_for_done(ctrl, pmic_arb->wr_base, sid, 0);
>       raw_spin_unlock_irqrestore(&pmic_arb->lock, flags);
>  
>       return rc;
>  }
>  
> +/* Unsupported by HW */

this comemnt is useless.

> +static int
> +pmic_arb_non_data_cmd_v2(struct spmi_controller *ctrl, u8 opc, u8 sid)
> +{
> +     return -EOPNOTSUPP;
> +}
> +
> +/* Non-data command */
> +static int pmic_arb_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid)
> +{
> +     struct spmi_pmic_arb_dev *pmic_arb = spmi_controller_get_drvdata(ctrl);
> +
> +     pr_debug("op:0x%x sid:%d\n", opc, sid);

you should use dev_dbg.

> +
> +     /* Check for valid non-data command */
> +     if (opc < SPMI_CMD_RESET || opc > SPMI_CMD_WAKEUP)
> +             return -EINVAL;
> +
> +     return pmic_arb->ver_ops->non_data_cmd(ctrl, opc, sid);
> +}
> +
>  static int pmic_arb_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
>                            u16 addr, u8 *buf, size_t len)
>  {
> @@ -241,6 +302,7 @@ static int pmic_arb_read_cmd(struct spmi_controller 
> *ctrl, u8 opc, u8 sid,
>       u8 bc = len - 1;
>       u32 cmd;
>       int rc;
> +     phys_addr_t offset = pmic_arb->ver_ops->offset(pmic_arb, sid, addr);

u32? offset() op returns u32.

>  
>       if (bc >= PMIC_ARB_MAX_TRANS_BYTES) {
>               dev_err(&ctrl->dev,
> @@ -259,20 +321,20 @@ static int pmic_arb_read_cmd(struct spmi_controller 
> *ctrl, u8 opc, u8 sid,
>       else
>               return -EINVAL;
>  
> -     cmd = (opc << 27) | ((sid & 0xf) << 20) | (addr << 4) | (bc & 0x7);
> +     cmd = pmic_arb->ver_ops->fmt_cmd(opc, sid, addr, bc);
>  
>       raw_spin_lock_irqsave(&pmic_arb->lock, flags);
> -     pmic_arb_base_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
> -     rc = pmic_arb_wait_for_done(ctrl);
> +     pmic_arb_set_rd_cmd(pmic_arb, offset + PMIC_ARB_CMD, cmd);
> +     rc = pmic_arb_wait_for_done(ctrl, pmic_arb->rd_base, sid, addr);
>       if (rc)
>               goto done;
>  
> -     pa_read_data(pmic_arb, buf, PMIC_ARB_RDATA0(pmic_arb->channel),
> +     pa_read_data(pmic_arb, buf, offset + PMIC_ARB_RDATA0,
>                    min_t(u8, bc, 3));
>  
>       if (bc > 3)
>               pa_read_data(pmic_arb, buf + 4,
> -                             PMIC_ARB_RDATA1(pmic_arb->channel), bc - 4);
> +                             offset + PMIC_ARB_RDATA1, bc - 4);
>  
>  done:
>       raw_spin_unlock_irqrestore(&pmic_arb->lock, flags);
> @@ -287,6 +349,7 @@ static int pmic_arb_write_cmd(struct spmi_controller 
> *ctrl, u8 opc, u8 sid,
>       u8 bc = len - 1;
>       u32 cmd;
>       int rc;
> +     u32 offset = pmic_arb->ver_ops->offset(pmic_arb, sid, addr);
>  
>       if (bc >= PMIC_ARB_MAX_TRANS_BYTES) {
>               dev_err(&ctrl->dev,
> @@ -307,19 +370,19 @@ static int pmic_arb_write_cmd(struct spmi_controller 
> *ctrl, u8 opc, u8 sid,
>       else
>               return -EINVAL;
>  
> -     cmd = (opc << 27) | ((sid & 0xf) << 20) | (addr << 4) | (bc & 0x7);
> +     cmd = pmic_arb->ver_ops->fmt_cmd(opc, sid, addr, bc);
>  
>       /* Write data to FIFOs */
>       raw_spin_lock_irqsave(&pmic_arb->lock, flags);
> -     pa_write_data(pmic_arb, buf, PMIC_ARB_WDATA0(pmic_arb->channel)
> +     pa_write_data(pmic_arb, buf, offset + PMIC_ARB_WDATA0
>                                                       , min_t(u8, bc, 3));

coding style: comma should be on upper line

>       if (bc > 3)
>               pa_write_data(pmic_arb, buf + 4,
> -                             PMIC_ARB_WDATA1(pmic_arb->channel), bc - 4);
> +                             offset + PMIC_ARB_WDATA1, bc - 4);
>  
>       /* Start the transaction */
> -     pmic_arb_base_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
> -     rc = pmic_arb_wait_for_done(ctrl);
> +     pmic_arb_base_write(pmic_arb, offset + PMIC_ARB_CMD, cmd);
> +     rc = pmic_arb_wait_for_done(ctrl, pmic_arb->wr_base, sid, addr);
>       raw_spin_unlock_irqrestore(&pmic_arb->lock, flags);
>  
>       return rc;
> @@ -376,7 +439,7 @@ static void periph_interrupt(struct spmi_pmic_arb_dev 
> *pa, u8 apid)
>       u32 status;
>       int id;
>  
> -     status = readl_relaxed(pa->intr + SPMI_PIC_IRQ_STATUS(apid));
> +     status = readl_relaxed(pa->intr + pa->ver_ops->irq_status(apid));
>       while (status) {
>               id = ffs(status) - 1;
>               status &= ~(1 << id);
> @@ -402,7 +465,7 @@ static void pmic_arb_chained_irq(unsigned int irq, struct 
> irq_desc *desc)
>  
>       for (i = first; i <= last; ++i) {
>               status = readl_relaxed(intr +
> -                                    SPMI_PIC_OWNER_ACC_STATUS(pa->ee, i));
> +                                   pa->ver_ops->owner_acc_status(pa->ee, i));
>               while (status) {
>                       id = ffs(status) - 1;
>                       status &= ~(1 << id);
> @@ -422,7 +485,7 @@ static void qpnpint_irq_ack(struct irq_data *d)
>       u8 data;
>  
>       raw_spin_lock_irqsave(&pa->lock, flags);
> -     writel_relaxed(1 << irq, pa->intr + SPMI_PIC_IRQ_CLEAR(apid));
> +     writel_relaxed(1 << irq, pa->intr + pa->ver_ops->irq_clear(apid));
>       raw_spin_unlock_irqrestore(&pa->lock, flags);
>  
>       data = 1 << irq;
> @@ -439,10 +502,11 @@ static void qpnpint_irq_mask(struct irq_data *d)
>       u8 data;
>  
>       raw_spin_lock_irqsave(&pa->lock, flags);
> -     status = readl_relaxed(pa->intr + SPMI_PIC_ACC_ENABLE(apid));
> +     status = readl_relaxed(pa->intr + pa->ver_ops->acc_enable(apid));
>       if (status & SPMI_PIC_ACC_ENABLE_BIT) {
>               status = status & ~SPMI_PIC_ACC_ENABLE_BIT;
> -             writel_relaxed(status, pa->intr + SPMI_PIC_ACC_ENABLE(apid));
> +             writel_relaxed(status, pa->intr +
> +                            pa->ver_ops->acc_enable(apid));
>       }
>       raw_spin_unlock_irqrestore(&pa->lock, flags);
>  
> @@ -460,10 +524,10 @@ static void qpnpint_irq_unmask(struct irq_data *d)
>       u8 data;
>  
>       raw_spin_lock_irqsave(&pa->lock, flags);
> -     status = readl_relaxed(pa->intr + SPMI_PIC_ACC_ENABLE(apid));
> +     status = readl_relaxed(pa->intr + pa->ver_ops->acc_enable(apid));
>       if (!(status & SPMI_PIC_ACC_ENABLE_BIT)) {
>               writel_relaxed(status | SPMI_PIC_ACC_ENABLE_BIT,
> -                             pa->intr + SPMI_PIC_ACC_ENABLE(apid));
> +                             pa->intr + pa->ver_ops->acc_enable(apid));
>       }
>       raw_spin_unlock_irqrestore(&pa->lock, flags);
>  
> @@ -624,6 +688,91 @@ static int qpnpint_irq_domain_map(struct irq_domain *d,
>       return 0;
>  }
>  
> +/* v1 offset per ee */
> +static u32 pmic_arb_offset_v1(struct spmi_pmic_arb_dev *pa, u8 sid, u16 addr)
> +{
> +     return 0x800 + 0x80 * (pa->channel);

no braces here and in below ops

> +}
> +
> +/* v2 offset per ppid (chan) and per ee */
> +static u32 pmic_arb_offset_v2(struct spmi_pmic_arb_dev *pa, u8 sid, u16 addr)
> +{
> +     u16 ppid = (sid << 8) | (addr >> 8);
> +     u8  chan = pa->ppid_to_chan[ppid];
> +
> +     return 0x1000 * pa->ee + 0x8000 * chan;
> +}
> +
> +static u32 pmic_arb_fmt_cmd_v1(u8 opc, u8 sid, u16 addr, u8 bc)
> +{
> +     return (opc << 27) | ((sid & 0xf) << 20) | (addr << 4) | (bc & 0x7);
> +}
> +
> +static u32 pmic_arb_fmt_cmd_v2(u8 opc, u8 sid, u16 addr, u8 bc)
> +{
> +     return (opc << 27) | ((addr & 0xff) << 4) | (bc & 0x7);
> +}
> +
> +static u32 pmic_arb_owner_acc_status_v1(u8 m, u8 n)
> +{
> +     return 0x20 * (m) + 0x4 * (n);
> +}
> +
> +static u32 pmic_arb_owner_acc_status_v2(u8 m, u8 n)
> +{
> +     return 0x100000 + 0x1000 * (m) + 0x4 * (n);
> +}
> +
> +static u32 pmic_arb_acc_enable_v1(u8 n)
> +{
> +     return 0x200 + 0x4 * (n);
> +}
> +
> +static u32 pmic_arb_acc_enable_v2(u8 n)
> +{
> +     return 0x1000 * (n);
> +}
> +
> +static u32 pmic_arb_irq_status_v1(u8 n)
> +{
> +     return 0x600 + 0x4 * (n);
> +}
> +
> +static u32 pmic_arb_irq_status_v2(u8 n)
> +{
> +     return 0x4 + 0x1000 * (n);
> +}
> +
> +static u32 pmic_arb_irq_clear_v1(u8 n)
> +{
> +     return 0xA00 + 0x4 * (n);
> +}
> +
> +static u32 pmic_arb_irq_clear_v2(u8 n)
> +{
> +     return 0x8 + 0x1000 * (n);
> +}
> +
> +static const struct pmic_arb_ver_ops pmic_arb_v1 = {
> +     .non_data_cmd           = pmic_arb_non_data_cmd_v1,
> +     .offset                 = pmic_arb_offset_v1,
> +     .fmt_cmd                = pmic_arb_fmt_cmd_v1,
> +     .owner_acc_status       = pmic_arb_owner_acc_status_v1,
> +     .acc_enable             = pmic_arb_acc_enable_v1,
> +     .irq_status             = pmic_arb_irq_status_v1,
> +     .irq_clear              = pmic_arb_irq_clear_v1,
> +};
> +
> +static const struct pmic_arb_ver_ops pmic_arb_v2 = {
> +     .non_data_cmd           = pmic_arb_non_data_cmd_v2,
> +     .offset                 = pmic_arb_offset_v2,
> +     .fmt_cmd                = pmic_arb_fmt_cmd_v2,
> +     .owner_acc_status       = pmic_arb_owner_acc_status_v2,
> +     .acc_enable             = pmic_arb_acc_enable_v2,
> +     .irq_status             = pmic_arb_irq_status_v2,
> +     .irq_clear              = pmic_arb_irq_clear_v2,
> +};
> +
>  static const struct irq_domain_ops pmic_arb_irq_domain_ops = {
>       .map    = qpnpint_irq_domain_map,
>       .xlate  = qpnpint_irq_domain_dt_translate,
> @@ -634,8 +783,9 @@ static int spmi_pmic_arb_probe(struct platform_device 
> *pdev)
>       struct spmi_pmic_arb_dev *pa;
>       struct spmi_controller *ctrl;
>       struct resource *res;
> -     u32 channel, ee;
> +     u32 channel, ee, hw_ver;
>       int err, i;
> +     bool is_v1;
>  
>       ctrl = spmi_controller_alloc(&pdev->dev, sizeof(*pa));
>       if (!ctrl)
> @@ -645,12 +795,65 @@ static int spmi_pmic_arb_probe(struct platform_device 
> *pdev)
>       pa->spmic = ctrl;
>  
>       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
> -     pa->base = devm_ioremap_resource(&ctrl->dev, res);
> -     if (IS_ERR(pa->base)) {
> -             err = PTR_ERR(pa->base);
> +     pa->rd_base = devm_ioremap_resource(&ctrl->dev, res);
> +     if (IS_ERR(pa->rd_base)) {
> +             err = PTR_ERR(pa->rd_base);
>               goto err_put_ctrl;
>       }
>  
> +     hw_ver = readl_relaxed(pa->rd_base + PMIC_ARB_VERSION);
> +     is_v1  = (hw_ver < PMIC_ARB_VERSION_V2_MIN);
> +
> +     dev_info(&ctrl->dev, "PMIC Arb Version-%d (0x%x)\n", (is_v1 ? 1 : 2),
> +             hw_ver);
> +
> +     if (is_v1) {
> +             pa->ver_ops = &pmic_arb_v1;
> +             pa->wr_base = pa->rd_base;
> +     } else {
> +             u8  chan;
> +             u16 ppid;
> +             u32 regval;
> +
> +             pa->ver_ops = &pmic_arb_v2;
> +
> +             pa->ppid_to_chan = devm_kzalloc(&ctrl->dev,
> +                                     PPID_TO_CHAN_TABLE_SZ, GFP_KERNEL);
> +             if (!pa->ppid_to_chan) {
> +                     err = -ENOMEM;
> +                     goto err_put_ctrl;
> +             }
> +             /*
> +              * PMIC_ARB_REG_CHNL is a table in HW mapping channel to ppid.
> +              * ppid_to_chan is an in-memory invert of that table.
> +              */
> +             for (chan = 0; chan < PMIC_ARB_MAX_CHNL; ++chan) {
> +                     regval = readl_relaxed(pa->rd_base +
> +                                                   PMIC_ARB_REG_CHNL(chan));
> +                     if (!regval)
> +                             continue;
> +
> +                     ppid = (regval >> 8) & 0xFFF;
> +                     pa->ppid_to_chan[ppid] = chan;
> +             }
> +
> +             res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +                                                             "obsrvr");

coding style: "obsrvr" should start on the same coloumn as pdev. This
comment is valid to many places in this patch.

<snip>

I played a bit with slave device (RTC) on device with pmic arbiter v2,
so now the interrupts works.

-- 
regards,
Stan
--
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/

Reply via email to