Re: [PATCH] spmi: pmic_arb: add support for hw version 2
From: Gilad Avidov
Date: Fri Jan 23 2015 - 13:37:57 EST
Hi Ivan,
On 1/21/2015 7:32 AM, Ivan T. Ivanov wrote:
Hi Gilad,
Just few comments.
On Mon, 2015-01-19 at 18:10 -0700, Gilad Avidov wrote:
Qualcomm PMIC Arbiter version-2 changes from version-1 are:
- Some diffrent register offsets.
- New channel register space, one per PMIC peripheral (ppid).
All tx tarffic uses these channels.
- New observer register space. All rx trafic uses this space.
- Diffrent command format for spmi command registers.
Signed-off-by: Gilad Avidov <gavidov@xxxxxxxxxxxxxx>
Acked-by: Sagar Dharia <sdharia@xxxxxxxxxxxxxx>
---
<snip>
+struct pmic_arb_ver;
+
Is there a better name for this structure. Ok it contain version
information, but.
will rename to 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.
+ * @rd_base: on v1 "core", on v2 "chnls" register base off DT.
s/rd/wr/
* @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)
@@ -114,9 +121,11 @@ enum pmic_arb_cmd_op_code {
* @domain: irq domain object for PMIC IRQ domain
* @spmic: SPMI controller object
* @apid_to_ppid: cached mapping from APID to PPID
+ * @ppid_to_chan cached mapping from APID to channel number, 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_tlock;
@@ -129,17 +138,61 @@ struct spmi_pmic_arb_dev {
struct irq_domain*domain;
struct spmi_controller*spmic;
u16 apid_to_ppid[256];
+ const struct pmic_arb_ver *ver;
+ u8 *ppid_to_chan;
+};
+
+/**
+ * pmic_arb_ver: version dependent functionality and values.
+ *
+ * @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.
+ * @geni_ctrl: PMIC_ARB_GENI_CTRL offset.
+ * @geni_status: PMIC_ARB_GENI_STATUS offset.
+ * @protocol_irq_status: PMIC_ARB_PROTOCOL_IRQ_STATUS offset.
+ */
+struct pmic_arb_ver {
+ int (*non_data_cmd)(struct spmi_controller *ctrl, u8 opc, u8 sid);
+ /* SPMI commands (including data) related functionality */
+ off_t (*offset)(struct spmi_pmic_arb_dev *dev, u8 sid, u16 addr);
+ u32 (*fmt_cmd)(u8 opc, u8 sid, u16 addr, u8 bc);
+ /* Interrupts related functionality (offset of PIC registers) */
+ off_t (*owner_acc_status)(u8 m, u8 n);
+ off_t (*acc_enable)(u8 n);
+ off_t (*irq_status)(u8 n);
+ off_t (*irq_clear)(u8 n);
+ /* Register offsets */
+ off_t geni_ctrl;
+ off_t geni_status;
+ off_t protocol_irq_status;
};
<snip>
+
+/* 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);
Please use dev_dbg.
agree
+static const struct pmic_arb_ver pmic_arb_v1 = {
+ .non_data_cmd= pmic_arb_non_data_cmd_v1,
Missing space before =
will fix
+ .offset = pmic_arb_offset_v1,
+ .fmt_cmd = pmic_arb_fmt_cmd_v1,
Bad indentation... and bellow
+ .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,
+ .geni_ctrl= 0x24,
+ .geni_status= 0x28,
+ .protocol_irq_status= (0x700 + 0x820),
+};
+
+static const struct pmic_arb_ver 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,
+ .geni_ctrl= 0x28,
+ .geni_status= 0x2C,
+ .protocol_irq_status= (0x700 + 0x900),
+};
+
static const struct irq_domain_ops pmic_arb_irq_domain_ops = {
.map = qpnpint_irq_domain_map,
.xlate = qpnpint_irq_domain_dt_translate,
@@ -634,9 +798,11 @@ 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;
+ pr_err("PMIC Arbiter\n");
+
leftover from debug?
will remove.
ctrl = spmi_controller_alloc(&pdev->dev, sizeof(*pa));
if (!ctrl)
return -ENOMEM;
@@ -645,12 +811,64 @@ 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);
+
+ if (hw_ver < PMIC_ARB_VERSION_V2_MIN) {
+ pa->ver= &pmic_arb_v1;
+ dev_dbg(&ctrl->dev, "PMIC Arb Version-1 (0x%x)\n", hw_ver);
+ pa->wr_base = pa->rd_base;
+ } else {
+ u8 chan;
+ u16 ppid;
+ u32 regval;
+
+ pa->ver = &pmic_arb_v2;
+ dev_dbg(&ctrl->dev, "PMIC Arb Version-2 (0x%x)\n", hw_ver);
Do we really need two almost the same dev_dbg's?
I'll try to combine these two
+
+ pa->ppid_to_chan = devm_kzalloc(&ctrl->dev,
+ PPID_TO_CHAN_TABLE_SZ, GFP_KERNEL);
+ if (!pa->ppid_to_chan) {
+ dev_err(&ctrl->dev,
+ "faild allocation of ppid_to_chan table\n");
+ err = -ENOMEM;
+ goto err_put_ctrl;
+ }
+ /*
+ * PMIC_ARB_REG_CHNL is a table in HW mapping channel to ppid.
+ * ppid_to_chan is an inverted cache 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");
+ 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;
+ }
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ "chnls");
it looks like
pa->wr_base = devm_ioremap_resource(&ctrl->dev, res);
is missing?
correct, will add.
+ if (IS_ERR(pa->wr_base)) {
+ err = PTR_ERR(pa->wr_base);
+ goto err_put_ctrl;
+ }
+ }
+
Thanks Ivan.
Thanks Gilad
--
Qualcomm Innovation Center, Inc.
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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/