From: Amjad Ouled-Ameur <aouledameur@baylibre.com> To: Daniel Lezcano <daniel.lezcano@linaro.org> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, Fabien Parent <fparent@baylibre.com>, Matthias Brugger <matthias.bgg@gmail.com>, Markus Schneider-Pargmann <msp@baylibre.com>, linux-pm@vger.kernel.org, Rob Herring <robh@kernel.org>, Michael Kao <michael.kao@mediatek.com>, linux-kernel@vger.kernel.org, Hsin-Yi Wang <hsinyi@chromium.org>, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org Subject: Re: [PATCH v7 4/4] thermal: mediatek: add another get_temp ops for thermal sensors Date: Mon, 26 Dec 2022 11:27:33 +0100 [thread overview] Message-ID: <fa6ec5ae-3c09-6a8d-b6bd-46fbdfcb4f26@baylibre.com> (raw) In-Reply-To: <COUMF3IZ9Y63.LA3KFHJSUZIC@amjad-ThinkPad-T490> Hi Daniel, On 12/6/22 10:18, Amjad Ouled-Ameur wrote: > Hi Daniel, > On Mon Dec 5, 2022 at 8:39 PM CET, Daniel Lezcano wrote: >> Hi Amjad, >> >> >> On 05/12/2022 11:41, Amjad Ouled-Ameur wrote: >> >> [ ... ] >> >>>>> @@ -1161,11 +1197,24 @@ static int mtk_thermal_probe(struct platform_device *pdev) >>>>> >>>>> platform_set_drvdata(pdev, mt); >>>>> >>>>> - tzdev = devm_thermal_of_zone_register(&pdev->dev, 0, mt, >>>>> - &mtk_thermal_ops); >>>>> - if (IS_ERR(tzdev)) { >>>>> - ret = PTR_ERR(tzdev); >>>>> - goto err_disable_clk_peri_therm; >>>>> + for (i = 0; i < mt->conf->num_sensors + 1; i++) { >>>>> + tz = devm_kmalloc(&pdev->dev, sizeof(*tz), GFP_KERNEL); >>>>> + if (!tz) >>>>> + return -ENOMEM; >>>>> + >>>>> + tz->mt = mt; >>>>> + tz->id = i; >>>>> + >>>>> + tzdev = devm_thermal_of_zone_register(&pdev->dev, i, tz, (i == 0) ? >>>>> + &mtk_thermal_ops : >>>>> + &mtk_thermal_sensor_ops); >>>> Here you use again the aggregation >>> I addressed this concern in V6, could you please take a look and let me >>> know what you think [0]. >>> >>> [0]: https://lore.kernel.org/all/5eb0cdc2-e9f9-dd42-bf80-b7dcd8bcc196@baylibre.com/ >> May I misunderstanding but AFAICS, this patch is setting the >> mtk_thermal_ops if the sensor id is zero. The get_temp is computing the >> max temperature in this ops which is what we don't want to do. > Correct, but I think that is out of scope of this patchset, as the current > driver already uses mtk_thermal_ops for sensor 0. The focus of this patchset > is to add support for the other sensors. > > Besides, what do you suggest as a clean implementation if the current one > no longer meets thermal core requirements ? Could you please address this ? Kindly, Amjad > Regards, > Amjad >> >> -- >> <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
WARNING: multiple messages have this Message-ID (diff)
From: Amjad Ouled-Ameur <aouledameur@baylibre.com> To: Daniel Lezcano <daniel.lezcano@linaro.org> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, Fabien Parent <fparent@baylibre.com>, Matthias Brugger <matthias.bgg@gmail.com>, Markus Schneider-Pargmann <msp@baylibre.com>, linux-pm@vger.kernel.org, Rob Herring <robh@kernel.org>, Michael Kao <michael.kao@mediatek.com>, linux-kernel@vger.kernel.org, Hsin-Yi Wang <hsinyi@chromium.org>, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org Subject: Re: [PATCH v7 4/4] thermal: mediatek: add another get_temp ops for thermal sensors Date: Mon, 26 Dec 2022 11:27:33 +0100 [thread overview] Message-ID: <fa6ec5ae-3c09-6a8d-b6bd-46fbdfcb4f26@baylibre.com> (raw) In-Reply-To: <COUMF3IZ9Y63.LA3KFHJSUZIC@amjad-ThinkPad-T490> Hi Daniel, On 12/6/22 10:18, Amjad Ouled-Ameur wrote: > Hi Daniel, > On Mon Dec 5, 2022 at 8:39 PM CET, Daniel Lezcano wrote: >> Hi Amjad, >> >> >> On 05/12/2022 11:41, Amjad Ouled-Ameur wrote: >> >> [ ... ] >> >>>>> @@ -1161,11 +1197,24 @@ static int mtk_thermal_probe(struct platform_device *pdev) >>>>> >>>>> platform_set_drvdata(pdev, mt); >>>>> >>>>> - tzdev = devm_thermal_of_zone_register(&pdev->dev, 0, mt, >>>>> - &mtk_thermal_ops); >>>>> - if (IS_ERR(tzdev)) { >>>>> - ret = PTR_ERR(tzdev); >>>>> - goto err_disable_clk_peri_therm; >>>>> + for (i = 0; i < mt->conf->num_sensors + 1; i++) { >>>>> + tz = devm_kmalloc(&pdev->dev, sizeof(*tz), GFP_KERNEL); >>>>> + if (!tz) >>>>> + return -ENOMEM; >>>>> + >>>>> + tz->mt = mt; >>>>> + tz->id = i; >>>>> + >>>>> + tzdev = devm_thermal_of_zone_register(&pdev->dev, i, tz, (i == 0) ? >>>>> + &mtk_thermal_ops : >>>>> + &mtk_thermal_sensor_ops); >>>> Here you use again the aggregation >>> I addressed this concern in V6, could you please take a look and let me >>> know what you think [0]. >>> >>> [0]: https://lore.kernel.org/all/5eb0cdc2-e9f9-dd42-bf80-b7dcd8bcc196@baylibre.com/ >> May I misunderstanding but AFAICS, this patch is setting the >> mtk_thermal_ops if the sensor id is zero. The get_temp is computing the >> max temperature in this ops which is what we don't want to do. > Correct, but I think that is out of scope of this patchset, as the current > driver already uses mtk_thermal_ops for sensor 0. The focus of this patchset > is to add support for the other sensors. > > Besides, what do you suggest as a clean implementation if the current one > no longer meets thermal core requirements ? Could you please address this ? Kindly, Amjad > Regards, > Amjad >> >> -- >> <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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-12-26 10:27 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-11-18 11:04 [PATCH v7 0/4] thermal: mediatek: Add support for MT8365 SoC Amjad Ouled-Ameur 2022-11-18 11:04 ` Amjad Ouled-Ameur 2022-11-18 11:04 ` [PATCH v7 1/4] dt-bindings: thermal: mediatek: add binding documentation " Amjad Ouled-Ameur 2022-11-18 11:04 ` Amjad Ouled-Ameur 2022-11-18 11:04 ` [PATCH v7 2/4] thermal: mediatek: control buffer enablement tweaks Amjad Ouled-Ameur 2022-11-18 11:04 ` Amjad Ouled-Ameur 2022-11-18 11:04 ` [PATCH v7 3/4] thermal: mediatek: add support for MT8365 SoC Amjad Ouled-Ameur 2022-11-18 11:04 ` Amjad Ouled-Ameur 2022-11-18 11:04 ` [PATCH v7 4/4] thermal: mediatek: add another get_temp ops for thermal sensors Amjad Ouled-Ameur 2022-11-18 11:04 ` Amjad Ouled-Ameur 2022-12-04 17:26 ` Daniel Lezcano 2022-12-04 17:26 ` Daniel Lezcano 2022-12-05 10:41 ` Amjad Ouled-Ameur 2022-12-05 10:41 ` Amjad Ouled-Ameur 2022-12-05 19:39 ` Daniel Lezcano 2022-12-05 19:39 ` Daniel Lezcano 2022-12-06 9:18 ` Amjad Ouled-Ameur 2022-12-06 9:18 ` Amjad Ouled-Ameur 2022-12-26 10:27 ` Amjad Ouled-Ameur [this message] 2022-12-26 10:27 ` Amjad Ouled-Ameur 2022-12-29 15:49 ` Daniel Lezcano 2022-12-29 15:49 ` Daniel Lezcano 2023-01-19 17:03 ` Amjad Ouled-Ameur 2023-01-19 17:03 ` Amjad Ouled-Ameur 2023-01-24 10:08 ` Amjad Ouled-Ameur 2023-01-24 10:08 ` Amjad Ouled-Ameur 2023-01-24 16:54 ` Daniel Lezcano 2023-01-24 16:54 ` Daniel Lezcano 2023-01-24 17:46 ` Amjad Ouled-Ameur 2023-01-24 17:46 ` Amjad Ouled-Ameur 2023-01-24 17:55 ` Daniel Lezcano 2023-01-24 17:55 ` Daniel Lezcano 2023-01-24 22:27 ` Amjad Ouled-Ameur 2023-01-24 22:27 ` Amjad Ouled-Ameur 2023-01-25 10:02 ` Daniel Lezcano 2023-01-25 10:02 ` Daniel Lezcano 2023-01-25 10:27 ` Amjad Ouled-Ameur 2023-01-25 10:27 ` Amjad Ouled-Ameur
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=fa6ec5ae-3c09-6a8d-b6bd-46fbdfcb4f26@baylibre.com \ --to=aouledameur@baylibre.com \ --cc=angelogioacchino.delregno@collabora.com \ --cc=daniel.lezcano@linaro.org \ --cc=devicetree@vger.kernel.org \ --cc=fparent@baylibre.com \ --cc=hsinyi@chromium.org \ --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=michael.kao@mediatek.com \ --cc=msp@baylibre.com \ --cc=robh@kernel.org \ /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.