linux-kernel - Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

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: <9BE41927-7C6C-4E62-860B-0288627E9421@codeaurora.org>
Date:	Wed, 4 Sep 2013 11:42:49 -0500
From:	Kumar Gala <galak@...eaurora.org>
To:	Suman Anna <s-anna@...com>
Cc:	Ohad Ben-Cohen <ohad@...ery.com>, Tony Lindgren <tony@...mide.com>,
	Benoit Cousson <bcousson@...libre.com>,
	<linux-kernel@...r.kernel.org>, <linux-omap@...r.kernel.org>,
	<devicetree@...r.kernel.org>
Subject: Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes


On Sep 4, 2013, at 11:27 AM, Suman Anna wrote:

> Kumar,
> 
>>>>>> 
>>>>>>> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
>>>>>>> which are all device-tree boot only. This patch adds the
>>>>>>> base support for parsing the DT nodes, and removes the code
>>>>>>> dealing with the traditional platform device instantiation.
>>>>>>> 
>>>>>>> Signed-off-by: Suman Anna <s-anna@...com>
>>>>>>> ---
>>>>>>> .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
>>>>>>> arch/arm/mach-omap2/Makefile                       |  3 --
>>>>>>> arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
>>>>>>> drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
>>>>>>> 4 files changed, 45 insertions(+), 67 deletions(-)
>>>>>>> create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>> delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
>>>>>>> 
>>>>>>> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>> new file mode 100644
>>>>>>> index 0000000..adfb8ad
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>> @@ -0,0 +1,28 @@
>>>>>>> +OMAP4+ HwSpinlock Driver
>>>>>>> +
>>>>>>> +Required properties:
>>>>>>> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
>>>>>>> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
>>>>>>> +- reg:			Contains the hwspinlock register address range (base
>>>>>>> +			address and length)
>>>>>>> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
>>>>>>> +
>>>>>>> +Optional properties:
>>>>>>> +- base_id:		Base Id for the locks for a particular hwspinlock
>>>>>>> +			device. If not mentioned, a default value of 0 is used.
>>>>>>> +			This property is mandatory ONLY if a SoC has several
>>>>>>> +			hwspinlock devices. There are currently no such OMAP
>>>>>>> +			SoCs.
>>>>>> 
>>>>>> Should this be ti,base_id ? [ I know its kinda generic in its intent for any SoC w/multiple blocks ]
>>>>> 
>>>>> I didn't add the "ti," prefix exactly for the same reason - it is
>>>>> generic w.r.t the hwspinlock core irrespective of the SoC family, and
>>>>> there is nothing ti or OMAP specific about it. I have added it to keep
>>>>> the DT node definition in sync with the driver code. If it is too
>>>>> generic a name, it can always be renamed as hwlock_base_id. This will be
>>>>> SoC agnostic property for the hwspinlock driver. What do you think?
>>>> 
>>>> I'm wondering if we should use cell-index for this purpose.
>>> 
>>> I didn't get you completely. Do you intend to compute the base_id using
>>> cell-index and number of locks (which may be a separate field altogether
>>> if this information cannot be read from the h/w)? My understanding is
>>> that cell-index is primarily used for identifying the h/w instance number.
>> 
>> I was suggesting using cell-index instead of base_id.  What we should probably due is have a devicetree/bindings/hwlock/hwlock.txt that would describe generic properties like this and just reference that in the omap binding spec.
> 
> Common hwlock.txt sounds good. Will make the change.
> 
>> 
>> I'm thinking if we dont use cell-index, that it should probably be hwlock-base-id
>> 
> 
> I prefer to use hwlock-base-id. I think we should also be defining a
> common property name for number of locks, say hwlock-num-locks.

I'm good with that, cell-index is always funny so might as well be explicit.

I'm also good with hwlock-num-locks, I'll update the msm spinlock driver to use this.

Can you also maybe add some helper functions into the hwspinlock core to return these values so we both don't duplicate code in drivers and maintain consistency.

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/