lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150203212150.GE4855@linaro.org>
Date:	Tue, 3 Feb 2015 14:21:50 -0700
From:	Lina Iyer <lina.iyer@...aro.org>
To:	Stanimir Varbanov <svarbanov@...sol.com>
Cc:	Gilad Avidov <gavidov@...eaurora.org>, sdharia@...eaurora.org,
	mlocke@...eaurora.org, linux-arm-msm@...r.kernel.org,
	gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
	iivanov@...sol.com, galak@...eaurora.org, agross@...eaurora.org
Subject: Re: [PATCH V2 2/2] spmi: pmic_arb: add support for hw version 2

On Tue, Feb 03 2015 at 02:59 -0700, Stanimir Varbanov wrote:
>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 <gavidov@...eaurora.org>
>> Acked-by: Sagar Dharia <sdharia@...eaurora.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.

I dont see any DOS line endings with these patches. I believe the
checkpatch warnings also are false positives.

Thanks,
Lina

>
>>   *
>>   * 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-arm-msm" in
>the body of a message to majordomo@...r.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ