From: Eduardo Valentin <edubezval@gmail.com> To: Sascha Hauer <s.hauer@pengutronix.de> Cc: linux-pm@vger.kernel.org, Zhang Rui <rui.zhang@intel.com>, linux-kernel@vger.kernel.org, kernel@pengutronix.de, linux-mediatek@lists.infradead.org, linux-arm-kernel@lists.infradead.org, Matthias Brugger <matthias.bgg@gmail.com> Subject: Re: [PATCH 2/3] thermal: Add Mediatek thermal controller support Date: Thu, 17 Dec 2015 11:33:33 -0800 [thread overview] Message-ID: <20151217193332.GB7999@localhost.localdomain> (raw) In-Reply-To: <1448883753-19068-3-git-send-email-s.hauer@pengutronix.de> Sascha, Yeah, sorry for the long delay. I was planing on applying this patch for the next merge window, but it just came across one point, see below. On Mon, Nov 30, 2015 at 12:42:32PM +0100, Sascha Hauer wrote: > This adds support for the Mediatek thermal controller found on MT8173 > +static const int sensor_mux_values[MT8173_NUM_SENSORS] = { 0, 1, 2, 3, 16 }; > + <big cut> > +/* > + * The MT8173 thermal controller has four banks. Each bank can read up to > + * four temperature sensors simultaneously. The MT8173 has a total of 5 > + * temperature sensors. We use each bank to measure a certain area of the > + * SoC. Since TS2 is located centrally in the SoC it is influenced by multiple > + * areas, hence is used in different banks. > + * > + * The thermal core only gets the maximum temperature of all banks, so > + * the bank concept wouldn't be necessary here. However, the SVS (Smart > + * Voltage Scaling) unit makes its decisions based on the same bank > + * data, and this indeed needs the temperatures of the individual banks > + * for making better decisions. > + */ > +static const struct mtk_thermal_bank_cfg bank_data[] = { > + { > + .num_sensors = 2, > + .sensors = { MT8173_TS2, MT8173_TS3 }, > + }, { > + .num_sensors = 2, > + .sensors = { MT8173_TS2, MT8173_TS4 }, > + }, { > + .num_sensors = 3, > + .sensors = { MT8173_TS1, MT8173_TS2, MT8173_TSABB }, > + }, { > + .num_sensors = 1, > + .sensors = { MT8173_TS2 }, > + }, > +}; Why can't we expose all these as thermal zones? That should remove the policy of computing the maximum from this driver. Please have a look on the work being done [1] to add grouping and aggregation of thermal zones. With that in place, you should be a matter of configuring the grouping and selecting max as the aggregation function, from the thermal core, instead in the driver. Which should give the system engineer, more flexibility to compose whatever policy based on the exposed sensors. BR, Eduardo Valentin [1] - https://lkml.org/lkml/2015/11/25/446
WARNING: multiple messages have this Message-ID (diff)
From: edubezval@gmail.com (Eduardo Valentin) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 2/3] thermal: Add Mediatek thermal controller support Date: Thu, 17 Dec 2015 11:33:33 -0800 [thread overview] Message-ID: <20151217193332.GB7999@localhost.localdomain> (raw) In-Reply-To: <1448883753-19068-3-git-send-email-s.hauer@pengutronix.de> Sascha, Yeah, sorry for the long delay. I was planing on applying this patch for the next merge window, but it just came across one point, see below. On Mon, Nov 30, 2015 at 12:42:32PM +0100, Sascha Hauer wrote: > This adds support for the Mediatek thermal controller found on MT8173 > +static const int sensor_mux_values[MT8173_NUM_SENSORS] = { 0, 1, 2, 3, 16 }; > + <big cut> > +/* > + * The MT8173 thermal controller has four banks. Each bank can read up to > + * four temperature sensors simultaneously. The MT8173 has a total of 5 > + * temperature sensors. We use each bank to measure a certain area of the > + * SoC. Since TS2 is located centrally in the SoC it is influenced by multiple > + * areas, hence is used in different banks. > + * > + * The thermal core only gets the maximum temperature of all banks, so > + * the bank concept wouldn't be necessary here. However, the SVS (Smart > + * Voltage Scaling) unit makes its decisions based on the same bank > + * data, and this indeed needs the temperatures of the individual banks > + * for making better decisions. > + */ > +static const struct mtk_thermal_bank_cfg bank_data[] = { > + { > + .num_sensors = 2, > + .sensors = { MT8173_TS2, MT8173_TS3 }, > + }, { > + .num_sensors = 2, > + .sensors = { MT8173_TS2, MT8173_TS4 }, > + }, { > + .num_sensors = 3, > + .sensors = { MT8173_TS1, MT8173_TS2, MT8173_TSABB }, > + }, { > + .num_sensors = 1, > + .sensors = { MT8173_TS2 }, > + }, > +}; Why can't we expose all these as thermal zones? That should remove the policy of computing the maximum from this driver. Please have a look on the work being done [1] to add grouping and aggregation of thermal zones. With that in place, you should be a matter of configuring the grouping and selecting max as the aggregation function, from the thermal core, instead in the driver. Which should give the system engineer, more flexibility to compose whatever policy based on the exposed sensors. BR, Eduardo Valentin [1] - https://lkml.org/lkml/2015/11/25/446
next prev parent reply other threads:[~2015-12-17 19:33 UTC|newest] Thread overview: 164+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-11-30 11:42 [PATCH v12] Add Mediatek thermal support Sascha Hauer 2015-11-30 11:42 ` Sascha Hauer 2015-11-30 11:42 ` [PATCH 1/3] dt-bindings: thermal: Add binding document for Mediatek thermal controller Sascha Hauer 2015-11-30 11:42 ` Sascha Hauer 2015-12-16 11:23 ` Daniel Kurtz 2015-12-16 11:23 ` Daniel Kurtz 2015-12-16 11:23 ` Daniel Kurtz 2015-12-17 19:23 ` Eduardo Valentin 2015-12-17 19:23 ` Eduardo Valentin 2015-12-17 19:23 ` Eduardo Valentin 2015-12-18 7:16 ` Sascha Hauer 2015-12-18 7:16 ` Sascha Hauer 2015-12-18 7:16 ` Sascha Hauer 2015-12-20 2:12 ` Eduardo Valentin 2015-12-20 2:12 ` Eduardo Valentin 2015-12-20 2:12 ` Eduardo Valentin 2015-11-30 11:42 ` [PATCH 2/3] thermal: Add Mediatek thermal controller support Sascha Hauer 2015-11-30 11:42 ` Sascha Hauer 2015-12-17 19:33 ` Eduardo Valentin [this message] 2015-12-17 19:33 ` Eduardo Valentin 2016-01-04 14:19 ` Sascha Hauer 2016-01-04 14:19 ` Sascha Hauer 2016-01-19 7:29 ` Sascha Hauer 2016-01-19 7:29 ` Sascha Hauer 2016-02-01 2:54 ` Eddie Huang 2016-02-01 2:54 ` Eddie Huang 2016-02-01 2:54 ` Eddie Huang 2016-02-15 2:11 ` Daniel Kurtz 2016-02-15 2:11 ` Daniel Kurtz 2016-02-15 2:11 ` Daniel Kurtz 2016-02-15 2:14 ` Daniel Kurtz 2016-02-15 2:14 ` Daniel Kurtz 2016-02-15 2:14 ` Daniel Kurtz 2016-02-17 17:05 ` Matthias Brugger 2016-02-17 17:05 ` Matthias Brugger 2016-02-17 17:05 ` Matthias Brugger 2016-02-18 10:56 ` Sascha Hauer 2016-02-18 10:56 ` Sascha Hauer 2016-02-18 10:56 ` Sascha Hauer 2016-02-18 14:28 ` Javi Merino 2016-02-18 14:28 ` Javi Merino 2016-02-18 14:28 ` Javi Merino 2016-02-18 15:15 ` Eduardo Valentin 2016-02-18 15:15 ` Eduardo Valentin 2016-02-18 15:15 ` Eduardo Valentin 2016-02-19 7:21 ` Sascha Hauer 2016-02-19 7:21 ` Sascha Hauer 2016-02-19 7:21 ` Sascha Hauer 2015-12-21 4:07 ` Daniel Kurtz 2015-12-21 4:07 ` Daniel Kurtz 2015-12-21 4:07 ` Daniel Kurtz 2016-01-04 14:31 ` Sascha Hauer 2016-01-04 14:31 ` Sascha Hauer 2016-01-04 14:31 ` Sascha Hauer 2016-01-04 15:43 ` Daniel Kurtz 2016-01-04 15:43 ` Daniel Kurtz 2016-01-04 15:43 ` Daniel Kurtz 2015-11-30 11:42 ` [PATCH 3/3] ARM64: dts: mt8173: Add thermal/auxadc device nodes Sascha Hauer 2015-11-30 11:42 ` Sascha Hauer 2016-02-18 16:18 ` Matthias Brugger 2016-02-18 16:18 ` Matthias Brugger 2016-02-19 7:20 ` Sascha Hauer 2016-02-19 7:20 ` Sascha Hauer 2016-04-20 11:23 ` Matthias Brugger 2016-04-20 11:23 ` Matthias Brugger 2015-12-14 10:37 ` [PATCH v12] Add Mediatek thermal support Sascha Hauer 2015-12-14 10:37 ` Sascha Hauer 2015-12-14 10:37 ` Sascha Hauer 2015-12-14 21:08 ` Eduardo Valentin 2015-12-14 21:08 ` Eduardo Valentin 2016-02-18 15:50 ` [PATCH 1/1] thermal: small style cleanup in mtk_thermal Eduardo Valentin 2016-02-18 15:50 ` Eduardo Valentin 2016-02-18 15:56 ` Matthias Brugger 2016-02-18 15:56 ` Matthias Brugger 2016-02-18 15:53 ` [PATCH v12] Add Mediatek thermal support Eduardo Valentin 2016-02-18 15:53 ` Eduardo Valentin -- strict thread matches above, loose matches on Subject: below -- 2015-11-18 8:24 [PATCH v11] " Sascha Hauer 2015-11-18 8:24 ` [PATCH 2/3] thermal: Add Mediatek thermal controller support Sascha Hauer 2015-11-18 8:24 ` Sascha Hauer 2015-11-24 6:06 ` dawei chien 2015-11-24 6:06 ` dawei chien 2015-11-24 6:06 ` dawei chien 2015-11-24 7:53 ` Sascha Hauer 2015-11-24 7:53 ` Sascha Hauer 2015-11-09 10:13 [PATCH v10] Add Mediatek thermal support Sascha Hauer 2015-11-09 10:13 ` [PATCH 2/3] thermal: Add Mediatek thermal controller support Sascha Hauer 2015-11-09 10:13 ` Sascha Hauer 2015-11-09 10:13 ` Sascha Hauer 2015-11-09 14:39 ` Andy Shevchenko 2015-11-09 14:39 ` Andy Shevchenko 2015-11-18 8:11 ` Sascha Hauer 2015-11-18 8:11 ` Sascha Hauer 2015-11-18 8:11 ` Sascha Hauer 2015-11-10 12:05 ` Javi Merino 2015-11-10 12:05 ` Javi Merino 2015-11-10 18:26 ` Eduardo Valentin 2015-11-10 18:26 ` Eduardo Valentin 2015-11-11 7:27 ` Sascha Hauer 2015-11-11 7:27 ` Sascha Hauer 2015-11-11 9:40 ` Javi Merino 2015-11-11 9:40 ` Javi Merino 2015-11-11 9:40 ` Javi Merino 2015-11-13 10:09 ` Sascha Hauer 2015-11-13 10:09 ` Sascha Hauer 2015-11-13 11:26 ` Javi Merino 2015-11-13 11:26 ` Javi Merino 2015-11-18 8:18 ` Sascha Hauer 2015-11-18 8:18 ` Sascha Hauer 2015-09-23 13:37 [PATCH v9] Add Mediatek thermal support Sascha Hauer 2015-09-23 13:37 ` [PATCH 2/3] thermal: Add Mediatek thermal controller support Sascha Hauer 2015-09-23 13:37 ` Sascha Hauer 2015-09-23 18:31 ` Vladimir Zapolskiy 2015-09-23 18:31 ` Vladimir Zapolskiy 2015-09-23 18:31 ` Vladimir Zapolskiy 2015-09-30 6:14 ` Sascha Hauer 2015-09-30 6:14 ` Sascha Hauer 2015-09-29 23:04 ` Eduardo Valentin 2015-09-29 23:04 ` Eduardo Valentin 2015-09-29 23:04 ` Eduardo Valentin 2015-09-30 6:13 ` Sascha Hauer 2015-09-30 6:13 ` Sascha Hauer 2015-09-30 9:36 ` Punit Agrawal 2015-09-30 9:36 ` Punit Agrawal 2015-09-30 9:36 ` Punit Agrawal 2015-09-30 10:37 ` Sascha Hauer 2015-09-30 10:37 ` Sascha Hauer 2015-09-30 11:07 ` Punit Agrawal 2015-09-30 11:07 ` Punit Agrawal 2015-08-31 7:34 [PATCH v8] Add Mediatek thermal support Sascha Hauer 2015-08-31 7:34 ` [PATCH 2/3] thermal: Add Mediatek thermal controller support Sascha Hauer 2015-08-31 7:34 ` Sascha Hauer 2015-08-31 7:34 ` Sascha Hauer 2015-09-14 7:32 ` Daniel Kurtz 2015-09-14 7:32 ` Daniel Kurtz 2015-09-14 7:32 ` Daniel Kurtz 2015-09-22 7:30 ` Daniel Kurtz 2015-09-22 7:30 ` Daniel Kurtz 2015-09-22 7:30 ` Daniel Kurtz 2015-09-22 8:30 ` Sascha Hauer 2015-09-22 8:30 ` Sascha Hauer 2015-09-22 8:30 ` Sascha Hauer 2015-08-27 6:41 [PATCH v7] Add Mediatek thermal support Sascha Hauer 2015-08-27 6:41 ` [PATCH 2/3] thermal: Add Mediatek thermal controller support Sascha Hauer 2015-08-27 11:50 ` Punit Agrawal 2015-08-26 13:58 [PATCH v6] Add Mediatek thermal support Sascha Hauer 2015-08-26 13:58 ` [PATCH 2/3] thermal: Add Mediatek thermal controller support Sascha Hauer 2015-08-20 8:05 [PATCH v5] Add Mediatek thermal support Sascha Hauer 2015-08-20 8:06 ` [PATCH 2/3] thermal: Add Mediatek thermal controller support Sascha Hauer 2015-08-20 22:20 ` Eduardo Valentin 2015-08-20 22:29 ` Daniel Lezcano 2015-08-21 5:06 ` Sascha Hauer 2015-08-20 23:12 ` Daniel Lezcano 2015-08-26 13:54 ` Sascha Hauer 2015-08-26 14:02 ` Daniel Lezcano 2015-08-25 17:41 ` Daniel Kurtz 2015-08-07 13:55 [PATCH v4] Add Mediatek thermal support Sascha Hauer 2015-08-07 13:55 ` [PATCH 2/3] thermal: Add Mediatek thermal controller support Sascha Hauer 2015-08-11 7:03 ` Daniel Kurtz 2015-08-20 7:57 ` Sascha Hauer 2015-08-05 12:25 [PATCH v3] Add Mediatek thermal support Sascha Hauer 2015-08-05 12:25 ` [PATCH 2/3] thermal: Add Mediatek thermal controller support Sascha Hauer 2015-08-05 18:02 ` Daniel Kurtz 2015-08-06 8:10 ` Sascha Hauer 2015-07-21 7:59 [PATCH v2] Add Mediatek thermal support Sascha Hauer 2015-07-21 7:59 ` [PATCH 2/3] thermal: Add Mediatek thermal controller support Sascha Hauer 2015-07-21 7:59 ` Sascha Hauer 2015-07-21 15:13 ` Daniel Kurtz 2015-07-21 15:13 ` Daniel Kurtz 2015-07-21 15:13 ` Daniel Kurtz 2015-08-05 10:20 ` Sascha Hauer 2015-08-05 10:20 ` Sascha Hauer 2015-08-05 10:20 ` Sascha Hauer 2015-07-13 10:34 [PATCH] thermal: Add Mediatek thermal support Sascha Hauer 2015-07-13 10:34 ` [PATCH 2/3] thermal: Add Mediatek thermal controller support Sascha Hauer 2015-07-13 10:34 ` Sascha Hauer
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 \ --in-reply-to=20151217193332.GB7999@localhost.localdomain \ --to=edubezval@gmail.com \ --cc=kernel@pengutronix.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mediatek@lists.infradead.org \ --cc=linux-pm@vger.kernel.org \ --cc=matthias.bgg@gmail.com \ --cc=rui.zhang@intel.com \ --cc=s.hauer@pengutronix.de \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.