linux-kernel - Re: [PATCH v2 1/2] soc: qcom: provide mechanism for drivers to access L2 registers

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]

Date:	Fri, 5 Aug 2016 14:53:47 -0400
From:	Neil Leeder <nleeder@...eaurora.org>
To:	Mark Rutland <mark.rutland@....com>
Cc:	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	Mark Langsdorf <mlangsdo@...hat.com>,
	Mark Salter <msalter@...hat.com>, Jon Masters <jcm@...hat.com>,
	Timur Tabi <timur@...eaurora.org>, cov@...eaurora.org,
	nleeder@...eaurora.org
Subject: Re: [PATCH v2 1/2] soc: qcom: provide mechanism for drivers to access
 L2 registers


On 8/5/2016 06:00 AM, Mark Rutland wrote:
> On Thu, Aug 04, 2016 at 05:11:10PM -0400, Neil Leeder wrote:
>> L2 registers are accessed using a select register and data
>> register pair. To prevent multiple concurrent writes to the
>> select register by independent drivers, the write to the
>> select register and the associated access of the data register
>> are protected with a lock. All drivers accessing the L2
>> registers use the set and get functions provided by
>> l2-accessors to ensure correct reads and writes to L2 registers.
> 
> As of this series, this is only used by the PMU driver. Which other
> drivers do you plan to use this for?
> 
> If there's nothing else planned at the moment, it would be nicer to fold
> these into the PMU driver.
> 

I see a couple of other drivers on codeaurora.org using it: the
Error Reporting (ERP) driver and an adaptive clock generator. 
I'd guess they'll be submitted to LKML but they're not mine so I don't
know when.

As the purpose of this is to be the common interface for multiple drivers
to stop them walking over each other, I think it makes sense to keep
it separate.

> [...]
> 
>> +config QCOM_L2_ACCESSORS
>> +	bool "Qualcomm Technologies L2-cache accessors"
>> +	depends on ARCH_QCOM && ARM64
>> +	help
>> +	  Say y here to enable support for the Qualcomm Technologies
>> +	  L2 accessors.
>> +	  Provides support for accessing registers in the L2 cache
>> +	  for Qualcomm Technologies chips.
> 
> Which chips have this?

Qualcomm Technologies ARM64 chips, so currently QDF24xx family and 
anything Kryo based. I'd assume any future chip families as well.
Given the 'depends on' line, I wasn't sure there was any benefit
to essentially duplicating that in the help text.

> 
> Have drivers select this as necessary. There's no reason for this to be
> used-selectable given this is trivial common infrastructure.

OK, I'll fix that

> 
> [...]
> 
>> +#include <linux/spinlock.h>
>> +#include <linux/export.h>
>> +#include <linux/soc/qcom/l2-accessors.h>
>> +#include <asm/cputype.h>
>> +#include <asm/sysreg.h>
> 
> Nit: please sort these alphabetically.

OK

> 
> [...]
> 
>> +EXPORT_SYMBOL(set_l2_indirect_reg);
> 
> The PMU driver isn't a module, so this doesn't need to be exported.
> Until there's a modular user, please get rid of EXPORT_SYMBOL.
> 
>> +EXPORT_SYMBOL(get_l2_indirect_reg);
> 
> Likewise.

OK to both of these.

> 
> Thanks,
> Mark.
> 

Thank you for the comments.
Neil

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.