On 2017-06-01 03:48, Stephen Boyd wrote:
> 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?
>
Sure. Will change it in the follow up patch.
>> +
>> 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.
>
When adding the new field description added extra tab for mode for
alignment.
That's why it is showing up.
>> * @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?
>
Yes. Just removed the extra space after '='.
>>
>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> "obsrvr");