From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: [PATCH V1 10/15] spmi: pmic_arb: add support for PMIC bus arbiter v3 Date: Wed, 31 May 2017 15:18:34 -0700 Message-ID: <20170531221834.GJ20170@codeaurora.org> References: <1496147943-25822-1-git-send-email-kgunda@codeaurora.org> <1496147943-25822-11-git-send-email-kgunda@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:42182 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751004AbdEaWSg (ORCPT ); Wed, 31 May 2017 18:18:36 -0400 Content-Disposition: inline In-Reply-To: <1496147943-25822-11-git-send-email-kgunda@codeaurora.org> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Kiran Gunda Cc: Abhijeet Dharmapurikar , David Collins , Christophe JAILLET , Subbaraman Narayanamurthy , linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, adharmap@quicinc.com, aghayal@qti.qualcomm.com On 05/30, Kiran Gunda wrote: > > /* PMIC Arbiter channel registers offsets */ > @@ -96,6 +97,17 @@ enum pmic_arb_cmd_op_code { > /* interrupt enable bit */ > #define SPMI_PIC_ACC_ENABLE_BIT BIT(0) > > +#define HWIRQ(slave_id, periph_id, irq_id, apid) \ > + ((((slave_id) & 0xF) << 28) | \ > + (((periph_id) & 0xFF) << 20) | \ > + (((irq_id) & 0x7) << 16) | \ > + (((apid) & 0x1FF) << 0)) > + > +#define HWIRQ_SID(hwirq) (((hwirq) >> 28) & 0xF) > +#define HWIRQ_PER(hwirq) (((hwirq) >> 20) & 0xFF) > +#define HWIRQ_IRQ(hwirq) (((hwirq) >> 16) & 0x7) > +#define HWIRQ_APID(hwirq) (((hwirq) >> 0) & 0x1FF) How about lowercase and hwirq_to_*() macros? Then it's more function like style. And spec_to_hwirq() or something? > + > struct pmic_arb_ver_ops; > > struct apid_data { > @@ -151,7 +163,9 @@ struct spmi_pmic_arb { > /** > * pmic_arb_ver: version dependent functionality. > * > - * @mode: access rights to specified pmic peripheral. > + * @ver_str: version string. > + * @ppid_to_apid: finds the apid for a given ppid. > + * @mode: access rights to specified pmic peripheral. mode didn't need to change tabbing. Not sure why it's appearing in the diff. > * @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. > @@ -177,10 +192,10 @@ struct pmic_arb_ver_ops { > 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); > + u32 (*owner_acc_status)(u8 m, u16 n); > + u32 (*acc_enable)(u16 n); > + u32 (*irq_status)(u16 n); > + u32 (*irq_clear)(u16 n); > }; > > static inline void pmic_arb_base_write(struct spmi_pmic_arb *pa, > @@ -776,7 +787,7 @@ static int qpnpint_irq_domain_map(struct irq_domain *d, > } > > static int > -pmic_arb_mode_v1(struct spmi_pmic_arb *pa, u8 sid, u16 addr, mode_t *mode) > +pmic_arb_mode_v1_v3(struct spmi_pmic_arb *pa, u8 sid, u16 addr, mode_t *mode) Nice to know that the mode became useless in v3 and beyond! Sigh. > { > *mode = S_IRUSR | S_IWUSR; > return 0; > @@ -987,21 +1017,21 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev) > > /* the apid to ppid table starts at PMIC_ARB_REG_CHNL(0) */ > - pa->max_periph = (pa->core_size - PMIC_ARB_REG_CHNL(0)) / 4; > + pa->max_periph = (pa->core_size - PMIC_ARB_REG_CHNL(0)) / 4; This looks unrelated? > > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, > "obsrvr"); -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project