From: Daniel Lezcano <firstname.lastname@example.org> To: Andy Tang <email@example.com>, "firstname.lastname@example.org" <email@example.com> Cc: "firstname.lastname@example.org" <email@example.com>, "firstname.lastname@example.org" <email@example.com>, "firstname.lastname@example.org" <email@example.com>, Rob Herring <firstname.lastname@example.org> Subject: Re: [PATCH] thermal: qoriq: add multiple sensors support Date: Wed, 24 Oct 2018 10:29:08 +0200 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <DB5PR0401MB221381AD65417D17AAD7B16DF3F60@DB5PR0401MB2213.eurprd04.prod.outlook.com> On 24/10/2018 04:48, Andy Tang wrote: > Hi Daniel, > >> -----Original Message----- >> From: Daniel Lezcano <firstname.lastname@example.org> >> Sent: 2018年10月16日 19:21 >> To: Andy Tang <email@example.com>; firstname.lastname@example.org >> Cc: email@example.com; firstname.lastname@example.org; >> email@example.com; Rob Herring <firstname.lastname@example.org> >> Subject: Re: [PATCH] thermal: qoriq: add multiple sensors support >> >>>>>> The current code is reading the DT in order to get the sensor id >>>>>> and initialize it. IOW, the DT gives the sensors to use. >>>>>> >>>>>> IMO, it would be more self contained if the driver initializes all >>>>>> the sensors without taking care of the DT and let the of- code to >>>>>> do the binding when the thermal zone, no ? >>>>> [Andy] could you please explain more about this way? I am not sure >>>>> how >>>> to implement it. >>>>> But one thing is for sure: we must get the sensor IDs explicitly so >>>>> that we can enable them by the following command: >> tmu_write(qdata, >>>>> sites | TMR_ME | TMR_ALPF, &qdata->regs->tmr); >>>> >>>> What I meant is about code separation between the driver itself and >>>> the of-thermal code. >>>> >>>> The code above re-inspect the DT to find out the sensor ids in order >>>> to enable them and somehow this is not wrong but breaks the self >>>> encapsulation of the driver. I was suggesting if it isn't possible to >>>> enable all the sensors without taking care of digging into the DT. >>> >>> [Andy] I don't want to re-parse the DT here too. But I have to. >>> This driver will be used by all our SOCs with different sensor IDs and >> number. >>> For example: there are 2 sensors on ls1088a platform with ID 0 and 1. >>> While on ls1043a there are 6 sensors with ID 0, 1, 2, 3, 4, 5. >>> If we don't scan the DT we would not know how many sensors it is and >>> what are the sensor's IDs, unless we hardcode it in driver. >> >> Yes, you are not the only one in this situation IMO and the drivers >> supporting multiple sensors are increasing, so this will repeat again and >> again. >> >> That could be hardcoded in the driver by using the compatible string but it >> will be nicer if we can fix that in the DT. >> >> [Cc'ing Rob] >> >> What is missing is a description of the sensors id in the temperature >> device node. We have the <thermal-sensor-cells> with 0 or 1 telling if >> there is one or several sensors but we can't specify which sensor ids we >> have. The only alternative is to parse the thermal zones to found out which >> sensors are in use and use them to initialize the driver, an approach which >> breaks the self-encapsulation: the of-thermal framework is the one in >> charge of doing the link between the thermal zone and a sensor id. >> >> Is it acceptable to add the list of the sensors id in the temp device node, so >> the driver can initialize these sensors without parsing the thermal zone in >> the DT ? >> > Have you got any conclusion yet? > When can I send the next version of this patch? Let's give an opportunity to Rob to answer. -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
next prev parent reply other threads:[~2018-10-24 8:29 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-09-27 2:42 andy.tang 2018-10-11 8:07 ` Andy Tang 2018-10-11 8:56 ` Daniel Lezcano 2018-10-13 20:42 ` Daniel Lezcano 2018-10-15 1:41 ` Andy Tang 2018-10-15 8:55 ` Daniel Lezcano 2018-10-16 3:01 ` Andy Tang 2018-10-16 11:20 ` Daniel Lezcano 2018-10-24 2:48 ` Andy Tang 2018-10-24 8:29 ` Daniel Lezcano [this message] 2018-10-24 12:52 ` Rob Herring
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH] thermal: qoriq: add multiple sensors support' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).