> -----Original Message----- > From: Eduardo Valentin <[email protected]> > Sent: 2018年11月30日 1:21 > To: Daniel Lezcano <[email protected]> > Cc: Andy Tang <[email protected]>; [email protected]; > [email protected]; [email protected] > Subject: Re: [PATCH v3] thermal: qoriq: add multiple sensors support > > On Wed, Nov 21, 2018 at 10:41:36AM +0100, Daniel Lezcano wrote: > > On 21/11/2018 10:16, 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-the > > > rmal 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. > > > > > > Something I missed? > > > > No. > > > > Eduardo, Rui, > > > > why not create a 'thermal' group ala 'tip' group with a single tree > > and two branches: > > > > thermal/next > > thermal/fixes > > > > - Rui takes the core changes. > > - Eduardo takes the SoC changes. > > > > - Both commit to thermal/next > > - Both commit to thermal/fixes > > - Both merge thermal/fixes into thermal/core as often as possible. > > > > That will help to have a more up to date branch, simplify the patch > > submission path and reduce the latency for the merge windows. > > > > If you need help, I can take care of applying the fixes only and merge > > them to thermal/next. > > > > That is how the tip subsystem works, Peter Ziljstra, Ingo Molnar, > > Thomas Gleixner, have all permissions to commit in the tip tree but > > they take care of their subsystems. If one is away for vacations or > > whatever, someone else can take over during the absence. > > > > Yeah, that is a setup people have been following. It does not necessarily mean > it will work for all cases though. > > I believe regardless of process and tree setup what we are lacking here is a > documentation of how things are being done. > > As I mentioned, I will work on writing something up to document at least what > we have today before any change in process gets in place. > [Andy] Besides the document, what about this patch? Could it be merged before the document is ready?
BR, Andy > > > > > > > > > > >> -----Original Message----- > > >> From: Daniel Lezcano <[email protected]> > > >> Sent: 2018年11月21日 16:44 > > >> To: Andy Tang <[email protected]>; [email protected]; > > >> [email protected] > > >> Cc: [email protected]; [email protected] > > >> 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-th > > >> ermal.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: [email protected]; [email protected] > > >>>> Cc: [email protected]; [email protected]; > > >>>> [email protected] > > >>>> Subject: RE: [PATCH v3] thermal: qoriq: add multiple sensors > > >>>> support > > >>>> > > >>>> PING. > > >>>> > > >>>> BR, > > >>>> Andy > > >>>> > > >>>>> -----Original Message----- > > >>>>> From: [email protected] <[email protected]> > > >>>>> Sent: 2018年10月30日 9:00 > > >>>>> To: [email protected]; [email protected] > > >>>>> Cc: [email protected]; [email protected]; > > >>>>> [email protected]; Andy Tang <[email protected]> > > >>>>> Subject: [PATCH v3] thermal: qoriq: add multiple sensors support > > >>>>> > > >>>>> From: Yuantian Tang <[email protected]> > > >>>>> > > >>>>> 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 <[email protected]> > > >>>>> Reviewed-by: Daniel Lezcano <[email protected]> > > >>>>> --- > > >>>>> 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 > > >> https://emea01.safelinks.protection.outlook.com/?url=www.linaro.org > > >> > &data=02%7C01%7Candy.tang%40nxp.com%7C523c8a2809a34f57700808d > 65 > > >> > 61f0575%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636791088572 > 03 > > >> > 0840&sdata=XV0I9%2B3FctUSF%2BDAqRVEWGk8ITzshd%2FtQfpRZJguYZc > %3D > > >> > &reserved=0%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 > > >> https://emea01.safelinks.protection.outlook.com/?url=www.facebook.c > > >> > om&data=02%7C01%7Candy.tang%40nxp.com%7C523c8a2809a34f577008 > 08d > > >> > 6561f0575%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6367910885 > 72 > > >> > 030840&sdata=bbsFbY4CoFGFyduTSXlR9oK42hpFGAXTmZMAswNchkU%3 > D& > > >> ;reserved=0%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%2F > > >> t > 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 > > >> https://emea01.safelinks.protection.outlook.com/?url=www.linaro.org > > >> > &data=02%7C01%7Candy.tang%40nxp.com%7C523c8a2809a34f57700808d > 65 > > >> > 61f0575%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636791088572 > 03 > > >> > 0840&sdata=XV0I9%2B3FctUSF%2BDAqRVEWGk8ITzshd%2FtQfpRZJguYZc > %3D > > >> > &reserved=0%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 > > > > > > > > > -- > > > > > <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww > > .linaro.org%2F&data=02%7C01%7Candy.tang%40nxp.com%7C523c8a280 > 9a34f > > > 57700808d6561f0575%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6 > 36791 > > > 088572030840&sdata=JdPh7CGtYKckvXZbaXyYh1YIzLIWuINxGuc8%2Fp5qb > 7w%3 > > D&reserved=0> Linaro.org │ Open source software for ARM SoCs > > > > Follow Linaro: > > > <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww > > .facebook.com%2Fpages%2FLinaro&data=02%7C01%7Candy.tang%40nx > p.com% > > > 7C523c8a2809a34f57700808d6561f0575%7C686ea1d3bc2b4c6fa92cd99c5c3016 > 35% > > > 7C0%7C0%7C636791088572030840&sdata=120jlpoyO%2FErgYCdiJkQ5PIu1 > lqIO > > 976W%2BBlifjq9zw%3D&reserved=0> Facebook | > > <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwi > > > tter.com%2F%23!%2Flinaroorg&data=02%7C01%7Candy.tang%40nxp.com > %7C5 > > > 23c8a2809a34f57700808d6561f0575%7C686ea1d3bc2b4c6fa92cd99c5c301635 > %7C0 > > %7C0%7C636791088572030840&sdata=hqRnoNwph7It3VtREb9PMnktZn > 6IWqPVht > > RqXL8qyKA%3D&reserved=0> Twitter | > > > <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww > > .linaro.org%2Flinaro-blog%2F&data=02%7C01%7Candy.tang%40nxp.com > %7C > > > 523c8a2809a34f57700808d6561f0575%7C686ea1d3bc2b4c6fa92cd99c5c301635 > %7C > > > 0%7C0%7C636791088572030840&sdata=GYax5%2FZzQtx6omIyuMMw0klh > rFUNsnP > > LFZ%2Fsg4OY%2BYI%3D&reserved=0> Blog > >
