Re: [PATCH v10 1/2] thermal: loongson-2: add thermal management support

[Christophe JAILLET]

Le 11/11/2022 à 11:05, Yinbo Zhu a écrit :
> 在 2022/11/10 下午2:01, Christophe JAILLET 写道:
> > Le 09/11/2022 à 09:09, Yinbo Zhu a écrit :
> > > Hi Rafael J.Wysocki,
> > >
> > > I just have a verified on your tree about my patch again, it is okay,
> > > if no other proplems, please help merge it to your tree and sync it 
> > > to linux mainline tree.
> > >
> > > Thanks,
> > > Yinbo.
> >
> > Hi,
> >
> > I've sent a few nits on v10, that you are obviously free to take into 
> > account or not.
> >
> > However, one of these comments is about thermal_add_hwmon_sysfs() vs 
> > devm_thermal_add_hwmon_sysfs() in loongson2_thermal_probe().
> >
> > IMHO, if I'm correct, at least this comment should be fixed.
> Hi Christophe JAILLET,
>
> Thanks your addvice, I had send v11 that following your advice,
> but some it is different. e.g.  devm_request_threaded_irq return value 
> is integer and use IS_ERR(integer) that isn't appropriate.

Hi,
thanks for taking my comments into consideration.


My point was something like:

	ret = devm_request_threaded_irq(dev, data->irq,
			loongson2_thermal_alarm_irq, loongson2_thermal_irq_thread,
			IRQF_ONESHOT, "loongson2_thermal", data);
	if (ret < 0)
		return dev_err_probe(dev, ret, "failed to request alarm irq\n");

But that  mostly a matter of taste. I don't think it worth a re-spin 
only for that.


However, I'll send another comment about the use of min() and max() in 
v11. I think it is not correct.

CJ

> Tks,
> Yinbo
> > CJ