| 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
| ||
|
Date: Tue, 1 Sep 2015 17:01:21 +0530
From: Kishon Vijay Abraham I <kishon@...com>
To: Mark Brown <broonie@...nel.org>, Lee Jones <lee.jones@...aro.org>,
Arnd Bergmann <arnd@...db.de>
CC: Grygorii Strashko <grygorii.strashko@...com>, <tony@...mide.com>,
<lgirdwood@...il.com>, <linux-omap@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <nsekhar@...com>,
Tero Kristo <t-kristo@...com>
Subject: Re: [PATCH 1/2] regulator: pbias: use untranslated address to program
pbias regulator
Now fixed Lee Jones mail address!
On Tuesday 01 September 2015 03:10 PM, Kishon Vijay Abraham I wrote:
> Hi Mark,
>
> On Monday 31 August 2015 08:22 PM, Mark Brown wrote:
>> On Mon, Aug 31, 2015 at 04:14:07PM +0530, Kishon Vijay Abraham I wrote:
>>> On Tuesday 25 August 2015 07:20 PM, Mark Brown wrote:
>>>> On Tue, Aug 25, 2015 at 01:03:04PM +0300, Grygorii Strashko wrote:
>>>>> On 08/19/2015 09:11 PM, Mark Brown wrote:
>>
>>>>>> So substract this address from the start of the resource to get the
>>>>>> offset? Or provide a wrapper function in the resource code which does
>>>>>> that.
>>>>
>>>>> I'd be very appreciated if you have and can share any thought on
>>>>> How can we get this absolute base address to substract?
>>>>
>>>> Ask the syscon device for its resource? Or have it provide an absolute
>>>
>>> Even if we get the absolute address of syscon, we have to do the
>>> subtraction only for the newer dtbs (previous dtbs already have only the
>>> offset). Do you recommend adding a new property to differentiate between
>>> older dtbs and newer dtbs? Any other suggestions here?
>>
>> Hang on. This is the first I've heard of any DTs not just having
>> absolute addresses. How does any of this work - has it ever worked, and
>> is the situation completely understood? My original concern with this
>
> Before commit d919501feffa8715147582c3ffce96fad0c7016f ARM: dts: dra7:
> add minimal l4 bus layout with control module support, the dt was like
>
> ocp {
> dra7_ctrl_general: tisyscon@...02e00 {
> compatible = "syscon";
> reg = <0x4a002e00 0x7c>;
> };
>
> pbias_regulator: pbias_regulator {
> compatible = "ti,pbias-omap";
> reg = <0 0x4>;
> syscon = <&dra7_ctrl_general>;
> };
> };
>
> Here platform_get_resource in pbias driver returns '0' which is
> populated in vsel_reg and enable_reg. And regulator_enable_regmap uses
> the base address from tisyscon@...02e00 which is '0x4a002e00' and offset
> from enable_reg which is '0' inorder to write to the pbias register.
>
> But after commit d919501fef, the dt became like this (after a couple of
> fixes)
>
> ocp {
> l4_cfg: l4@...00000 {
> compatible = "ti,dra7-l4-cfg", "simple-bus";
> ranges = <0 0x4a000000 0x22c000>;
>
> scm: scm@...0 {
> compatible = "ti,dra7-scm-core", "simple-bus";
> reg = <0x2000 0x2000>;
> ranges = <0 0x2000 0x2000>;
>
> scm_conf: scm_conf@0 {
> compatible = "syscon", "simple-bus";
> reg = <0x0 0x1400>;
> ranges = <0 0x0 0x1400>;
>
> pbias_regulator: pbias_regulator {
> compatible = "ti,pbias-omap";
> reg = <0xe00 0x4>;
> syscon = <&scm_conf>;
> };
> };
> };
> };
> };
>
> Here platform_get_resource in pbias driver returns '4a002e00' which is
> populated in vsel_reg and enable_reg. And regulator_enable_regmap uses
> the base address from scm_conf@0 which is '0x4a002000' and offset from
> enable_reg which is '4a002e00' inorder to write to the pbias register
> and it results in a abort.
>
>> was that I coudn't understand the commit log and your original response
>> seemed to indicate that we always have the absolute address :( Perhaps
>
> We started having the absolute address only after the dt change (commit
> d919501fef and a couple of more dt fixes).
>
>> this is something to do with the brief mention of having moved the DT
>> node for some reason?
>>
>> We at least need some sort of coherent explanation of the problem and a
>> comprehensible fix to go with it. Right now it seems like things are
>> just being moved about to hide problems without either of these things
>> which seems like it makes the code less clear and more fragile.
>
> hmm.. IMO the actual problem was modelling the 'offset' as a resource
> (by populating the offset in 'reg' property) which was added when the
> initial pbias dt node was created. And now since the pbias dt node is
> being moved around, it's causing inadvertent address translation
> breaking the pbias driver. IMHO the value in 'reg' property of pbias dt
> node should be treated as 'offset' instead of address 'resource' and
> that's what my patch tried to do.
>>
>>>> address based interface for that matter?
>>
>>> Syscon doesn't directly expose any API's to write to it's register.
>>> Rather it uses regmap APIs to read/write to it's register. I'm not sure
>>> if it's possible to add regmap APIs to write to a register with absolute
>>> address. Any hints?
>>
>> Yes, I'm aware that it is regmap based! What I am suggesting is that if
>> people are making DTs like yours with devices that are children of the
>> syscon then presumably it might be useful for it to allow client drivers
>> to pass absolute addresses in so that it can translate for them.
>
> Arnd, Lee?
>
> Thanks
> Kishon
>
--
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/
Powered by blists - more mailing lists