Re: [PATCH v3] thermal: qoriq: add multiple sensors support

From: Eduardo Valentin
Date: Thu Nov 29 2018 - 12:17:39 EST

Next message: Steven Rostedt: "Re: [PATCH v2 4/4] x86/static_call: Add inline static call implementation for x86-64"
Previous message: Peter Zijlstra: "Re: [PATCH v2 4/4] x86/static_call: Add inline static call implementation for x86-64"
Next in thread: Eduardo Valentin: "Re: [PATCH v3] thermal: qoriq: add multiple sensors support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Nov 21, 2018 at 09:16:08AM +0000, Andy Tang wrote:
> Hi Daniel,
>
> Thanks for your explanation. The problem is these two trees are not synced well.
> Let's take our driver(qoriq_thermal.c) for example.
>
> Git log on Rui's tree next branch:
> 2dfef65 thermal: qoriq: Switch to SPDX identifier
> 1a893a5 thermal: qoriq: Simplify the 'site' variable assignment
> f1506a6 thermal: qoriq: Use devm_thermal_zone_of_sensor_register()
> c30d5d5 thermal: qoriq: constify thermal_zone_of_device_ops structures
> 0e77488 thermal: qoriq: remove useless call for of_thermal_get_trip_points()
> 4352844 thermal: qoriq: Add thermal management support
>
> Git log on linux-soc-thermal tree branch next:
> 6017e2a thermal: qoriq: add i.mx8mq support
> 9b96566 thermal: Convert to using %pOFn instead of device_node.name
> c30d5d5 thermal: qoriq: constify thermal_zone_of_device_ops structures
> 0e77488 thermal: qoriq: remove useless call for of_thermal_get_trip_points()
> 4352844 thermal: qoriq: Add thermal management support
>
> You can see that the first 2-3 commits on these two tress are different.
>
> The strange thing is they seems sync well on Linus' tree:
> 0ef7791 Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal
> 6017e2a thermal: qoriq: add i.mx8mq support
> 9b96566 thermal: Convert to using %pOFn instead of device_node.name
> 2dfef65 thermal: qoriq: Switch to SPDX identifier
> 1a893a5 thermal: qoriq: Simplify the 'site' variable assignment
> f1506a6 thermal: qoriq: Use devm_thermal_zone_of_sensor_register()
> c30d5d5 thermal: qoriq: constify thermal_zone_of_device_ops structures
> 0e77488 thermal: qoriq: remove useless call for of_thermal_get_trip_points()
> 4352844 thermal: qoriq: Add thermal management support
>
> Currently my patch was created based on Run's tree, probably I should rebase it to soc tree.
> But whichever tree I use, it can't be merged to Linus' tree without conflict.

I agree that trees out of sync does not help. However the idea was to
have then separate, since couple of merge windows ago. But I believe the issue
here is that we do not have some sort of cookbook describing the
subsystem process.

I will work on describing it and sending a patch to have it documented.

>
> Something I missed?
>
> BR,
> Andy
> > -----Original Message-----
> > From: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> > Sent: 2018å11æ21æ 16:44
> > To: Andy Tang <andy.tang@xxxxxxx>; rui.zhang@xxxxxxxxx;
> > edubezval@xxxxxxxxx
> > Cc: linux-pm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH v3] thermal: qoriq: add multiple sensors support
> >
> > On 21/11/2018 02:34, Andy Tang wrote:
> > > Hi all,
> > >
> > > Do you have any comments on this patch?
> > >
> > > I found for our thermal driver(qoriq_thermal.c) there are different
> > between the following two git trees:
> > > git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git
> > > branch: next
> > >
> > git://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.gi
> > t.
> > > branch: next
> > >
> > > Could you please clarify which git tree/branch should I use?
> >
> > SoC changes are submitted against linux-soc-thermal.git.
> >
> > Generic thermal framework are sent against Zhang Rui's tree but it
> > happens sometimes Eduardo pick them also when the changes are related
> > to SoC behavior.
> >
> > However, I agree that can be confusing :)
> >
> > Eduardo, Rui,
> >
> > how about to add a section in the maintainer handbook for the thermal to
> > clarify the expectations and the flow?
> >
> > >> -----Original Message-----
> > >> From: Andy Tang
> > >> Sent: 2018å11æ14æ 15:29
> > >> To: rui.zhang@xxxxxxxxx; daniel.lezcano@xxxxxxxxxx
> > >> Cc: edubezval@xxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx;
> > >> linux-kernel@xxxxxxxxxxxxxxx
> > >> Subject: RE: [PATCH v3] thermal: qoriq: add multiple sensors support
> > >>
> > >> PING.
> > >>
> > >> BR,
> > >> Andy
> > >>
> > >>> -----Original Message-----
> > >>> From: andy.tang@xxxxxxx <andy.tang@xxxxxxx>
> > >>> Sent: 2018å10æ30æ 9:00
> > >>> To: rui.zhang@xxxxxxxxx; daniel.lezcano@xxxxxxxxxx
> > >>> Cc: edubezval@xxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx;
> > >>> linux-kernel@xxxxxxxxxxxxxxx; Andy Tang <andy.tang@xxxxxxx>
> > >>> Subject: [PATCH v3] thermal: qoriq: add multiple sensors support
> > >>>
> > >>> From: Yuantian Tang <andy.tang@xxxxxxx>
> > >>>
> > >>> The QorIQ Layerscape SoC has several thermal sensors but the
> > current
> > >>> driver only supports one.
> > >>>
> > >>> Massage the code to be sensor oriented and allow the support for
> > >>> multiple sensors.
> > >>>
> > >>> Signed-off-by: Yuantian Tang <andy.tang@xxxxxxx>
> > >>> Reviewed-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> > >>> ---
> > >>> v3:
> > >>> - add Reviewed-by
> > >>> v2:
> > >>> - update the commit message
> > >>> - refine the qoriq_tmu_register_tmu_zone()
> > >>>
> > >>> drivers/thermal/qoriq_thermal.c | 100
> > >>> ++++++++++++++++++---------------------
> > >>> 1 files changed, 46 insertions(+), 54 deletions(-)
> > >>>
> > >>> diff --git a/drivers/thermal/qoriq_thermal.c
> > >>> b/drivers/thermal/qoriq_thermal.c index 450ed66..8beb344 100644
> > >>> --- a/drivers/thermal/qoriq_thermal.c
> > >>> +++ b/drivers/thermal/qoriq_thermal.c
> > >>> @@ -59,14 +59,21 @@ struct qoriq_tmu_regs {
> > >>> u32 ttr3cr; /* Temperature Range 3 Control Register */
> > >>> };
> > >>>
> > >>> +struct qoriq_tmu_data;
> > >>> +
> > >>> /*
> > >>> * Thermal zone data
> > >>> */
> > >>> +struct qoriq_sensor {
> > >>> + struct thermal_zone_device *tzd;
> > >>> + struct qoriq_tmu_data *qdata;
> > >>> + int id;
> > >>> +};
> > >>> +
> > >>> struct qoriq_tmu_data {
> > >>> - struct thermal_zone_device *tz;
> > >>> struct qoriq_tmu_regs __iomem *regs;
> > >>> - int sensor_id;
> > >>> bool little_endian;
> > >>> + struct qoriq_sensor *sensor[SITES_MAX];
> > >>> };
> > >>>
> > >>> static void tmu_write(struct qoriq_tmu_data *p, u32 val, void
> > >>> __iomem
> > >>> *addr) @@ -87,48 +94,51 @@ static u32 tmu_read(struct
> > >> qoriq_tmu_data
> > >>> *p, void __iomem *addr)
> > >>>
> > >>> static int tmu_get_temp(void *p, int *temp) {
> > >>> + struct qoriq_sensor *qsensor = p;
> > >>> + struct qoriq_tmu_data *qdata = qsensor->qdata;
> > >>> u32 val;
> > >>> - struct qoriq_tmu_data *data = p;
> > >>>
> > >>> - val = tmu_read(data, &data->regs->site[data->sensor_id].tritsr);
> > >>> + val = tmu_read(qdata, &qdata->regs->site[qsensor->id].tritsr);
> > >>> *temp = (val & 0xff) * 1000;
> > >>>
> > >>> return 0;
> > >>> }
> > >>>
> > >>> -static int qoriq_tmu_get_sensor_id(void)
> > >>> +static const struct thermal_zone_of_device_ops tmu_tz_ops = {
> > >>> + .get_temp = tmu_get_temp,
> > >>> +};
> > >>> +
> > >>> +static int qoriq_tmu_register_tmu_zone(struct platform_device
> > >>> +*pdev)
> > >>> {
> > >>> - int ret, id;
> > >>> - struct of_phandle_args sensor_specs;
> > >>> - struct device_node *np, *sensor_np;
> > >>> + struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
> > >>> + int id, sites = 0;
> > >>>
> > >>> - np = of_find_node_by_name(NULL, "thermal-zones");
> > >>> - if (!np)
> > >>> - return -ENODEV;
> > >>> + for (id = 0; id < SITES_MAX; id++) {
> > >>> + qdata->sensor[id] = devm_kzalloc(&pdev->dev,
> > >>> + sizeof(struct qoriq_sensor), GFP_KERNEL);
> > >>> + if (!qdata->sensor[id])
> > >>> + return -ENOMEM;
> > >>>
> > >>> - sensor_np = of_get_next_child(np, NULL);
> > >>> - ret = of_parse_phandle_with_args(sensor_np,
> > "thermal-sensors",
> > >>> - "#thermal-sensor-cells",
> > >>> - 0, &sensor_specs);
> > >>> - if (ret) {
> > >>> - of_node_put(np);
> > >>> - of_node_put(sensor_np);
> > >>> - return ret;
> > >>> - }
> > >>> + qdata->sensor[id]->id = id;
> > >>> + qdata->sensor[id]->qdata = qdata;
> > >>>
> > >>> - if (sensor_specs.args_count >= 1) {
> > >>> - id = sensor_specs.args[0];
> > >>> - WARN(sensor_specs.args_count > 1,
> > >>> - "%s: too many cells in sensor specifier %d\n",
> > >>> - sensor_specs.np->name,
> > sensor_specs.args_count);
> > >>> - } else {
> > >>> - id = 0;
> > >>> - }
> > >>> + qdata->sensor[id]->tzd =
> > >>> devm_thermal_zone_of_sensor_register(
> > >>> + &pdev->dev, id, qdata->sensor[id], &tmu_tz_ops);
> > >>> + if (IS_ERR(qdata->sensor[id]->tzd)) {
> > >>> + if (PTR_ERR(qdata->sensor[id]->tzd) == -ENODEV)
> > >>> + continue;
> > >>> + else
> > >>> + return PTR_ERR(qdata->sensor[id]->tzd);
> > >>>
> > >>> - of_node_put(np);
> > >>> - of_node_put(sensor_np);
> > >>> + }
> > >>> +
> > >>> + sites |= 0x1 << (15 - id);
> > >>> + }
> > >>> + /* Enable monitoring */
> > >>> + if (sites != 0)
> > >>> + tmu_write(qdata, sites | TMR_ME | TMR_ALPF,
> > >>> &qdata->regs->tmr);
> > >>>
> > >>> - return id;
> > >>> + return 0;
> > >>> }
> > >>>
> > >>> static int qoriq_tmu_calibration(struct platform_device *pdev) @@
> > >>> -178,16 +188,11 @@ static void qoriq_tmu_init_device(struct
> > >>> qoriq_tmu_data *data)
> > >>> tmu_write(data, TMR_DISABLE, &data->regs->tmr); }
> > >>>
> > >>> -static const struct thermal_zone_of_device_ops tmu_tz_ops = {
> > >>> - .get_temp = tmu_get_temp,
> > >>> -};
> > >>> -
> > >>> static int qoriq_tmu_probe(struct platform_device *pdev) {
> > >>> int ret;
> > >>> struct qoriq_tmu_data *data;
> > >>> struct device_node *np = pdev->dev.of_node;
> > >>> - u32 site;
> > >>>
> > >>> if (!np) {
> > >>> dev_err(&pdev->dev, "Device OF-Node is NULL"); @@
> > -203,13
> > >>> +208,6 @@ static int qoriq_tmu_probe(struct platform_device
> > *pdev)
> > >>>
> > >>> data->little_endian = of_property_read_bool(np, "little-endian");
> > >>>
> > >>> - data->sensor_id = qoriq_tmu_get_sensor_id();
> > >>> - if (data->sensor_id < 0) {
> > >>> - dev_err(&pdev->dev, "Failed to get sensor id\n");
> > >>> - ret = -ENODEV;
> > >>> - goto err_iomap;
> > >>> - }
> > >>> -
> > >>> data->regs = of_iomap(np, 0);
> > >>> if (!data->regs) {
> > >>> dev_err(&pdev->dev, "Failed to get memory region\n");
> > @@
> > >>> -223,19 +221,13 @@ static int qoriq_tmu_probe(struct
> > platform_device
> > >>> *pdev)
> > >>> if (ret < 0)
> > >>> goto err_tmu;
> > >>>
> > >>> - data->tz = devm_thermal_zone_of_sensor_register(&pdev->dev,
> > >>> - data->sensor_id,
> > >>> - data, &tmu_tz_ops);
> > >>> - if (IS_ERR(data->tz)) {
> > >>> - ret = PTR_ERR(data->tz);
> > >>> - dev_err(&pdev->dev,
> > >>> - "Failed to register thermal zone device %d\n", ret);
> > >>> - goto err_tmu;
> > >>> + ret = qoriq_tmu_register_tmu_zone(pdev);
> > >>> + if (ret < 0) {
> > >>> + dev_err(&pdev->dev, "Failed to register sensors\n");
> > >>> + ret = -ENODEV;
> > >>> + goto err_iomap;
> > >>> }
> > >>>
> > >>> - /* Enable monitoring */
> > >>> - site = 0x1 << (15 - data->sensor_id);
> > >>> - tmu_write(data, site | TMR_ME | TMR_ALPF, &data->regs->tmr);
> > >>>
> > >>> return 0;
> > >>>
> > >>> --
> > >>> 1.7.1
> > >
> >
> >
> > --
> >
> > <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
> > www.linaro.org%2F&data=02%7C01%7Candy.tang%40nxp.com%7Ca0
> > d265f95a424d33256b08d64f8d74c8%7C686ea1d3bc2b4c6fa92cd99c5c
> > 301635%7C0%7C0%7C636783866299399290&sdata=AVw3XdjmiRO
> > XP7Xz7MTNMVgzI8hYG9aWsR02opMZqjM%3D&reserved=0>
> > Linaro.org â Open source software for ARM SoCs
> >
> > Follow Linaro:
> > <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
> > www.facebook.com%2Fpages%2FLinaro&data=02%7C01%7Candy.tan
> > g%40nxp.com%7Ca0d265f95a424d33256b08d64f8d74c8%7C686ea1d3b
> > c2b4c6fa92cd99c5c301635%7C0%7C0%7C636783866299399290&s
> > data=NM8wm4ri%2F2kdBW0FdCSXrL5ogg6owPfoRGacqm%2BKsEw%3D&a
> > mp;reserved=0> Facebook |
> > <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ft
> > witter.com%2F%23!%2Flinaroorg&data=02%7C01%7Candy.tang%40n
> > xp.com%7Ca0d265f95a424d33256b08d64f8d74c8%7C686ea1d3bc2b4c6
> > fa92cd99c5c301635%7C0%7C0%7C636783866299399290&sdata=
> > Wplqwpisgmqrf3yLhTzdo5O6TvN2Jfu5IjBTvS4xOWk%3D&reserved=0>
> > Twitter |
> > <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
> > www.linaro.org%2Flinaro-blog%2F&data=02%7C01%7Candy.tang%40
> > nxp.com%7Ca0d265f95a424d33256b08d64f8d74c8%7C686ea1d3bc2b4c
> > 6fa92cd99c5c301635%7C0%7C0%7C636783866299399290&sdata=I
> > b6KFIP3LNPGuDDb1dpnOllrqAAsc%2BZNCUuJZUPso6k%3D&reserve
> > d=0> Blog
>

Next message: Steven Rostedt: "Re: [PATCH v2 4/4] x86/static_call: Add inline static call implementation for x86-64"
Previous message: Peter Zijlstra: "Re: [PATCH v2 4/4] x86/static_call: Add inline static call implementation for x86-64"
Next in thread: Eduardo Valentin: "Re: [PATCH v3] thermal: qoriq: add multiple sensors support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]