linux-kernel - Re: [PATCH V1 10/15] spmi: pmic_arb: add support for PMIC bus arbiter v3

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: <20170531221834.GJ20170@codeaurora.org>
Date:   Wed, 31 May 2017 15:18:34 -0700
From:   Stephen Boyd <sboyd@...eaurora.org>
To:     Kiran Gunda <kgunda@...eaurora.org>
Cc:     Abhijeet Dharmapurikar <adharmap@...eaurora.org>,
        David Collins <collinsd@...eaurora.org>,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        Subbaraman Narayanamurthy <subbaram@...eaurora.org>,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        adharmap@...cinc.com, aghayal@....qualcomm.com
Subject: Re: [PATCH V1 10/15] spmi: pmic_arb: add support for PMIC bus
 arbiter v3

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