From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 29 Aug 2014 09:54:14 +0800 From: =?utf-8?B?6LW15Luq5bOw?= Subject: Re: Re: [PATCH v3 1/4] thermal: rockchip: add driver for thermal References: <1409187562-12370-1-git-send-email-caesar.wang@rock-chips.com>, <2852853.mfRZfFVSev@wuerfel>, <20140828143733.GA18084@developer>, <3440260.edal29kzlR@diego> Mime-Version: 1.0 Message-ID: <2014082909531345696848@rock-chips.com> Content-Type: multipart/alternative; boundary="----=_001_NextPart620851250586_=----" To: =?utf-8?B?SGVpa29TdMO8Ym5lcg==?= , Eduardo Valentin Cc: Arnd Bergmann , Caesar Wang , "rui.zhang" , "grant.likely" , robh+dt , linux-kernel , linux-pm , linux-arm-kernel , devicetree , linux-doc , =?utf-8?B?6buE5rab?= , =?utf-8?B?6JSh5p6r?= , dianders , dtor , =?utf-8?B?6ZKf5YuH5rGq?= , "addy.ke@rock-chips.com" , "dmitry.torokhov" , linux-iio , Jonathan Cameron List-ID: This is a multi-part message in MIME format. ------=_001_NextPart620851250586_=---- Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 SGkgSGVpa2/vvIwNCg0KICAgVGhlIFRTLUFEQyBvbiBSSzMyODggaGFzIHR3byBjb21wb25lbnQs IGEgdHNhZGMgYW5kIGEgdHNhZGMgY29udHJvbGxlci4gVGhlIHRzYWRjIGNvbnRyb2xsZXIgaXMg c2ltaWxhciBsaWtlIHRoZSB0aGVybWFsIG1hbmFnZXIgdW5pdCBvbiBvdGhlciBTT0NzLiBXZSBm b2xsb3dlZCB0aGUgbmFtaW5nIG9mIDMwNjYsIGJ1dCBub3QgbmFtZWQgYXMgdGhlIFRoZXJtYWwg TWFuYWdlci4gDQoNCiAgIE1vcmVvdmVyLHRoZXJlIGlzIG9ubHkgb25lIHNldCBvZiBhcGIgcmVn aXN0ZXJzIHRvIGFjY2VzcyB0aGUgdHNhZGMgY29udHJvbGxlcixhbmQgdGhlIHRzYWRjIGlzIGNv bnRyb2xsZWQgYnkgdGhlIHRzYWRjIGNvbnRyb2xsZXIsY291bGQgbm90IGFjY2VzcyBkaXJlY3Rs eS4gSWYgd2Ugd3JpdGUgYSBnZW5lcmFsIHRzYWRjIGRyaXZlciBieSBhY2Nlc3NlZCB0c2FkYyBj b250cm9sbGVyIHJlZ2lzdGVycywgYW5kIGl0IGhhcmRseSB0byB3cml0ZSBhIGRyaXZlciBmb3Ig dGhlIHRzYWRjIGNvbnRyb2xsZXIuIA0KDQogICBTbywgSSBkbyBub3QgYWdyZWUgdG8gd3JpdGUg YSBnZW5lcmljIGFkYyBkcml2ZXIgZm9yIHRoZSByazMyODgtdHNhZGMuDQoNCg0KDQpZaWZlbmcg Wmhhbw0KIA0K5Y+R5Lu25Lq677yaIEhlaWtvU3TDvGJuZXINCuWPkemAgeaXtumXtO+8miAyMDE0 LTA4LTI5IDAwOjExDQrmlLbku7bkurrvvJogRWR1YXJkbyBWYWxlbnRpbg0K5oqE6YCB77yaIEFy bmQgQmVyZ21hbm47IENhZXNhciBXYW5nOyBydWkuemhhbmc7IGdyYW50Lmxpa2VseTsgcm9iaCtk dDsgbGludXgta2VybmVsOyBsaW51eC1wbTsgbGludXgtYXJtLWtlcm5lbDsgZGV2aWNldHJlZTsg bGludXgtZG9jOyBodWFuZ3RhbzsgY2Y7IGRpYW5kZXJzOyBkdG9yOyB6eXc7IGFkZHkua2U7IGRt aXRyeS50b3Jva2hvdjsgemhhb3lpZmVuZzsgbGludXgtaWlvOyBKb25hdGhhbiBDYW1lcm9uDQrk uLvpopjvvJogUmU6IFtQQVRDSCB2MyAxLzRdIHRoZXJtYWw6IHJvY2tjaGlwOiBhZGQgZHJpdmVy IGZvciB0aGVybWFsDQogDQpBbSBEb25uZXJzdGFnLCAyOC4gQXVndXN0IDIwMTQsIDEwOjM3OjM1 IHNjaHJpZWIgRWR1YXJkbyBWYWxlbnRpbjoNCj4gQ2Vhc2FyIGFuZCBBcm5kLA0KPiANCj4gT24g VGh1LCBBdWcgMjgsIDIwMTQgYXQgMTA6NDg6MjNBTSArMDIwMCwgQXJuZCBCZXJnbWFubiB3cm90 ZToNCj4gPiBPbiBUaHVyc2RheSAyOCBBdWd1c3QgMjAxNCAwODo1OToxOSBDYWVzYXIgV2FuZyB3 cm90ZToNCj4gPiA+IFRoZXJtYWwgaXMgVFMtQURDIENvbnRyb2xsZXIgbW9kdWxlIHN1cHBvcnRz IHVzZXItZGVmaW5lZCBtb2RlIGFuZA0KPiA+ID4gYXV0b21hdGljIG1vZGUuDQo+ID4gPiANCj4g PiA+IFVzZXItZGVmaW5lZCBtb2RlIHJlZmVycyxUU0FEQyBhbGwgdGhlIGNvbnRyb2wgc2lnbmFs cyBlbnRpcmVseSBieQ0KPiA+ID4gc29mdHdhcmUNCj4gPiA+IHdyaXRpbmcgdG8gcmVnaXN0ZXIg Zm9yIGRpcmVjdCBjb250cm9sLg0KPiA+ID4gDQo+ID4gPiBBdXRvbWFpYyBtb2RlIHJlZmVycyB0 byB0aGUgbW9kdWxlIGF1dG9tYXRpY2FsbHkgcG9sbCBUU0FEQyBvdXRwdXQsYW5kDQo+ID4gPiB0 aGUgcmVzdWx0cyBXZXJlIGNoZWNrZWQuDQo+ID4gPiANCj4gPiA+IElmIHlvdSBmaW5kIHRoYXQg dGhlIHRlbXBlcmF0dXJlIEhpZ2ggaW4gYSBwZXJpb2Qgb2YgdGltZSwgYW4gaW50ZXJydXB0DQo+ ID4gPiBpcyBnZW5lcmF0ZWQgdG8gdGhlIHByb2Nlc3NvciBkb3duLW1lYXN1cmVzIHRha2VuO2lm IHRoZSB0ZW1wZXJhdHVyZQ0KPiA+ID4gb3ZlciBhIHBlcmlvZCBvZiB0aW1lIEhpZ2gsIHRoZSBy ZXN1bHRpbmcgVFNIVVQgZ2F2ZSBDUlUgbW9kdWxlLGxldCBpdA0KPiA+ID4gcmVzZXQgdGhlIGVu dGlyZSBjaGlwLCBvciB2aWEgR1BJTyBnaXZlIFBNSUMuDQo+ID4gPiANCj4gPiA+IFNpZ25lZC1v ZmYtYnk6IHpoYW95aWZlbmcgPHp5ZkByb2NrLWNoaXBzLmNvbT4NCj4gPiA+IFNpZ25lZC1vZmYt Ynk6IENhZXNhciBXYW5nIDxjYWVzYXIud2FuZ0Byb2NrLWNoaXBzLmNvbT4NCj4gPiANCj4gPiBI aSBDYWVzYXIsDQo+ID4gDQo+ID4gQWZ0ZXIgbG9va2luZyBhdCB0aGUgZHJpdmVyIChsYXN0IHRp bWUgSSBvbmx5IHJlY2VpdmVkIHRoZSBwYXRjaCBmb3INCj4gPiB0aGUgYmluZGluZyksIEkgaGF2 ZSBhIG1vcmUgZ2VuZXJhbCBjb21tZW50Og0KPiA+IA0KPiA+IFRoaXMgbG9va3MgbGlrZSBhIGdl bmVyYWwtcHVycG9zZSBBREMgZGV2aWNlLCBub3QgYW4gSVAgYmxvY2sgdGhhdCBpcw0KPiA+IHNw ZWNpZmljIHRvIHRoZXJtYWwgbWFuYWdlbWVudC4gVGhlIGJpbmRpbmcgbG9va3Mgb2sgZm9yIHRo YXQgcHVycG9zZQ0KPiA+IGJ1dCBzaG91bGQgcHJvYmFibHkgYmUgbW92ZWQgaW50bw0KPiA+IERv Y3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9paW8vYWRjLyBhcyBhIG1pbm9yIGNoYW5n ZS4NCj4gDQo+IEkgYWdyZWUgd2l0aCBBcm5kJ3MgcG9pbnQgaGVyZS4gSXQgbWFrZXMgc2Vuc2Ug dG8gbWUgdG8gaGF2ZSB0aGlzIGRyaXZlcg0KPiB1bmRlciB0aGUgSUlPIHVtYnJlbGxhLg0KIA0K aW50ZXJlc3Rpbmcgc3VnZ2VzdGlvbiA6LSkNCiANCkkndmUganVzdCB0YWtlbiBhbm90aGVyIGxv b2sgYXQgdGhlIHJlZ2lzdGVycyBvZiB0aGUgdHMtYWRjIG9uIHRoZSByazMwNjYgDQp3aGljaCBp cyBjb21wbGV0ZWx5IGRpZmZlcmVudCBmcm9tIHRoZSByazMyODggb25lLiBJbnRlcmVzdGluZ2x5 IHRoZSByazMwNjYgb25lIA0KaXMgImp1c3QiIGFub3RoZXIgc2FyYWRjIElQLCBzbyBmb3IgdGhp cyBvbmUgaXQgd291bGQgcmVhbGx5IG1ha2Ugc2Vuc2UuDQogDQogDQo+ID4gT24gdGhlIGRyaXZl ciBzaWRlLCBJIGJlbGlldmUgdGhlIGNvcnJlY3Qgd2F5IHRvIGRlYWwgd2l0aCB0aGlzIHNldHVw DQo+ID4gaXMgdG8gc3BsaXQgeW91ciBkcml2ZXIgaW50byBhIGdlbmVyaWMgZHJpdmVycy9paW8v YWRjL3JvY2tjaGlwcy10c2FkYy5jDQo+ID4gZmlsZSwgYW5kIGEgc21hbGxlciB0aGVybWFsIGRy aXZlciB0aGF0IHVzZXMgdGhlIGlpbyBpbi1rZXJuZWwgaW50ZXJmYWNlcywNCj4gPiBpZGVhbGx5 IG9uZSB0aGF0IGlzIGluZGVwZW5kZW50IG9mIHRoZSB1bmRlcmx5aW5nIGhhcmR3YXJlIGFuZCBj YW4NCj4gPiB3b3JrIG9uIGFueSBBREMgaW1wbGVtZW50YXRpb24uDQo+IA0KPiBBZ3JlZWQuIElm IHlvdSBjYW4gd3JpdGUgc3VjaCBpbnRlcmZhY2UgYW5kIG1ha2UgeW91ciBkcml2ZXIgdG8gd29y ayBpbg0KPiBzdWNoIHdheSwgdGhhdCB3b3VsZCBiZSBncmVhdC4NCiANCkJ1dCBJIGN1cnJlbnRs eSBkb24ndCBzZWUgaG93IHlvdSB3b3VsZCBtb2RlbCB0aGUgdGVtcGVyYXR1cmUgaGFuZGxpbmcg cGFydHMgDQpmcm9tIGEgZ2VuZXJpYyB0aGVybWFsIGRyaXZlciB0byBhIGdlbmVyaWMgYWRjIGRy aXZlciBmb3IgdGhlIHJrMzI4OC10c2FkYy4NCiANCkkgZ3Vlc3MgdGhlIGdlbmVyYWwgdGVtcGVy YXR1cmUgaXJxIGhhbmRsaW5nIHdvdWxkIHVzZSBpaW8tdHJpZ2dlcnM/IEJ1dCBob3cgDQpkb2Vz IHRoZSB0YXJnZXQgdGVtcGVyYXR1cmUgZ2V0IGludG8gdGhlIFRTQURDX0NPTVAxX0lOVCByZWdp c3Rlci4NCiANCkFsc28gd2hlbiBnZXR0aW5nIHRoZSB0ZW1wZXJhdHVyZSwgQ2Flc2FyJ3MgZHJp dmVyIGNvbXBhcmVzIGl0IHRvIGl0cyB0cmlwIA0KcG9pbnRzIGFuZCBzZXRzIHRoZSBuZXh0IHRy aXAgcG9pbnQgZGVwZW5kaW5nIG9uIHRoZSBjdXJyZW50IHRlbXBlcmF0dXJlIA0KKHBhc3NpdmUg PC0+IGNyaXRpY2FsKSBpbiByb2NrY2hpcF9nZXRfdGVtcC4NCiANCk1heWJlIHRoZXJlIGlzIHNv bWUgY29tcGxldGVseSBlYXN5IHdheSBmb3IgdGhpcywgYnV0IGN1cnJlbnRseSBJIGRvbid0IHNl ZSANCml0Lg0KIA0KIA0KSGVpa28NCiANCiANCg== ------=_001_NextPart620851250586_=---- Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable =0A
Hi Heiko=EF=BC=8C<= /div>

   The TS-ADC on RK3288 has two component, a tsadc and a tsad= c controller. The tsadc controller is similar like the thermal manager uni= t on other SOCs. We followed the naming of 3066, but not named as the Ther= mal Manager. 

   Moreover,there is only one set of apb regis= ters to access the tsadc controller,and the=0Atsadc is controlled by the t= sadc controller,could not access directly. If we write a general tsadc dri= ver by accessed tsadc controller registers, and it hardly to write a driv= er for the tsadc controller. 

  =  So, I do not agree to write a g= eneric adc driver for the rk3288-tsadc.

=0A
=0A=0A
Yifeng Zhao
 
=0A
=0A
=0A
=E5=8F=91=E4=BB= =B6=E4=BA=BA=EF=BC=9A HeikoSt= =C3=BCbner
=0A
=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4=EF=BC= =9A 2014-08-29 00:11
=0A
=E6=94=B6=E4=BB=B6=E4= =BA=BA=EF=BC=9A Eduardo Va= lentin
=0A=0A
=E4=B8=BB=E9=A2=98=EF=BC=9A Re: [PATCH v3 1/4] thermal: rockchip: add driver for thermal
<= /div>
=0A
 
=0A
Am Donnerstag, 28. August 2014= , 10:37:35 schrieb Eduardo Valentin:
=0A
> Ceasar and Arnd,=0A
>
=0A
> On Thu, Aug 28, 2014 at 10:48:23AM +020= 0, Arnd Bergmann wrote:
=0A
> > On Thursday 28 August 2014 = 08:59:19 Caesar Wang wrote:
=0A
> > > Thermal is TS-ADC = Controller module supports user-defined mode and
=0A
> > &g= t; automatic mode.
=0A
> > >
=0A
> > >= ; User-defined mode refers,TSADC all the control signals entirely by
= =0A
> > > software
=0A
> > > writing to reg= ister for direct control.
=0A
> > >
=0A
> &= gt; > Automaic mode refers to the module automatically poll TSADC outpu= t,and
=0A
> > > the results Were checked.
=0A
&= gt; > >
=0A
> > > If you find that the temperatur= e High in a period of time, an interrupt
=0A
> > > is ge= nerated to the processor down-measures taken;if the temperature
=0A> > > over a period of time High, the resulting TSHUT gave CRU= module,let it
=0A
> > > reset the entire chip, or via G= PIO give PMIC.
=0A
> > >
=0A
> > > Si= gned-off-by: zhaoyifeng <zyf@rock-chips.com>
=0A
> > = > Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
= =0A
> >
=0A
> > Hi Caesar,
=0A
> >= ;
=0A
> > After looking at the driver (last time I only re= ceived the patch for
=0A
> > the binding), I have a more ge= neral comment:
=0A
> >
=0A
> > This looks l= ike a general-purpose ADC device, not an IP block that is
=0A
>= ; > specific to thermal management. The binding looks ok for that purpo= se
=0A
> > but should probably be moved into
=0A
&= gt; > Documentation/devicetree/bindings/iio/adc/ as a minor change.=0A
>
=0A
> I agree with Arnd's point here. It makes= sense to me to have this driver
=0A
> under the IIO umbrella.=
=0A
 
=0A
interesting suggestion :-)
=0A
=  
=0A
I've just taken another look at the registers of the t= s-adc on the rk3066
=0A
which is completely different from the r= k3288 one. Interestingly the rk3066 one
=0A
is "just" another sa= radc IP, so for this one it would really make sense.
=0A
 =0A
 
=0A
> > On the driver side, I believe th= e correct way to deal with this setup
=0A
> > is to split y= our driver into a generic drivers/iio/adc/rockchips-tsadc.c
=0A
&= gt; > file, and a smaller thermal driver that uses the iio in-kernel in= terfaces,
=0A
> > ideally one that is independent of the un= derlying hardware and can
=0A
> > work on any ADC implement= ation.
=0A
>
=0A
> Agreed. If you can write such = interface and make your driver to work in
=0A
> such way, that= would be great.
=0A
 
=0A
But I currently don't se= e how you would model the temperature handling parts
=0A
from a = generic thermal driver to a generic adc driver for the rk3288-tsadc.
= =0A
 
=0A
I guess the general temperature irq handling w= ould use iio-triggers? But how
=0A
does the target temperature g= et into the TSADC_COMP1_INT register.
=0A
 
=0A
Als= o when getting the temperature, Caesar's driver compares it to its trip =0A
points and sets the next trip point depending on the current t= emperature
=0A
(passive <-> critical) in rockchip_get_temp= .
=0A
 
=0A
Maybe there is some completely easy way= for this, but currently I don't see
=0A
it.
=0A
 =
=0A
 
=0A
Heiko
=0A
 
=0A
&= nbsp;
=0A
=0A ------=_001_NextPart620851250586_=------