On 5/24/2016 07:23 AM, Mark Rutland wrote:
> On Mon, May 23, 2016 at 02:22:59PM -0400, Neil Leeder wrote:
>>
>> On 5/23/2016 01:25 PM, Mark Rutland wrote:
>>> On Fri, May 20, 2016 at 03:13:07PM -0400, Neil Leeder wrote:
>>>> Signed-off-by: Neil Leeder <nleeder@xxxxxxxxxxxxxx>
>>>> ---
>>>> drivers/soc/qcom/Kconfig | 9 +++++
>>>> drivers/soc/qcom/Makefile | 1 +
>>>> drivers/soc/qcom/l2-accessors.c | 66 +++++++++++++++++++++++++++++++++++
>>>> include/linux/soc/qcom/l2-accessors.h | 27 ++++++++++++++
>>>> 4 files changed, 103 insertions(+)
>>>> create mode 100644 drivers/soc/qcom/l2-accessors.c
>>>> create mode 100644 include/linux/soc/qcom/l2-accessors.h
>>>
>>> These are awfully generic file names (and function names). Which SoCs
>>> does this apply to?
>>>
>>> It would be good to give these more specific names.
>>
>> It's under soc/qcom, and dependent on ARCH_QCOM and (in v2) also on ARM64. It applies to all QCOM ARM64 SoCs.
>
> Per Christopher's comment, it sounds like this applies to QDF24xx.
>
> Given that the code uses IMPLEMENTATION DEFINED system registers, I
> presume that this does not apply to MSM8916 which uses Cortex-A53, for
> example (though perhaps it does, and I am mistaken).
>
>> Given that it can only be used in a QCOM driver, and the include path has qcom in it, I'd
>> prefer not to add redundancy by adding another qcom in there.
>
> I'm not asking for another "qcom", but simply the SoC variant or family
> (e.g. "qdf24xx" would be fine).
>
It applies to all ARMv8 SoCs with QCOM processors in them. So QDF24xx and mobile 820, but not SoCs
with ARM processors in them such as MSM8916. So neither msm_ nor qdf_ are accurate prefixes. As Timur
pointed out, the majority of source files in drivers/soc/qcom don't have any prefix, which is a
reason why I didn't include one.
>>>> diff --git a/include/linux/soc/qcom/l2-accessors.h b/include/linux/soc/qcom/l2-accessors.h
>>>> new file mode 100644
>>>> index 0000000..563c114
>>>> --- /dev/null
>>>> +++ b/include/linux/soc/qcom/l2-accessors.h
>>>> @@ -0,0 +1,27 @@
>>>> +/*
>>>> + * Copyright (c) 2011-2016 The Linux Foundation. All rights reserved.
>>>> + *
>>>> + * 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
>>>> + * only version 2 as published by the Free Software Foundation.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU General Public License for more details.
>>>> + */
>>>> +
>>>
>>>> +#ifndef __QCOM_L2_ACCESSORS_H
>>>> +#define __QCOM_L2_ACCESSORS_H
>>>> +
>>>> +#ifdef CONFIG_QCOM_L2_ACCESSORS
>>>> +void set_l2_indirect_reg(u64 reg_addr, u64 val);
>>>> +u64 get_l2_indirect_reg(u64 reg_addr);
>>>> +#else
>>>> +static inline void set_l2_indirect_reg(u64 reg_addr, u64 val) {}
>>>> +static inline u64 get_l2_indirect_reg(u64 reg_addr)
>>>> +{
>>>> + return 0;
>>>> +}
>>>
>>> Surely it would be better to error out on any unintentional use of these
>>> at build time?
>>
>> This allows building code which is common to ARM SoCs and QCOM SoCs without having to ifdef out the
>> QCOM-specific pieces.
>
> These shouldn't appear in generic code.
>
> Other than the L2 PMU driver (which presumably depends on or selects
> CONFIG_QCOM_L2_ACCESSORS), what code would you have to ifdef?
>
> I don't have a major concern on this, I just don't see where it should
> matter.
Ok, I agree, I will remove this. Thanks.
>
> Thanks,
> Mark.
>
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
