On 25 October 2012 16:41, Viresh Kumar <viresh.ku...@linaro.org> wrote:
> On 25 October 2012 13:56, Hongbo Zhang <hongbo.zh...@linaro.org> wrote:
>
> While replying to mails, don't remove lines like above. They help
> identifying who
> wrote what.
>
>> [...]
>>>> +/* Callback to get temperature changing trend */
>>>> +static int db8500_sys_get_trend(struct thermal_zone_device *thermal,
>
> For example, you can't tell who wrote this line...
>
>>>> +static int __devinit db8500_thermal_probe(struct platform_device *pdev)
>>>> +{
>>>> + struct db8500_thermal_zone *pzone = NULL;
>>>> + struct db8500_thsens_platform_data *ptrips = NULL;
>>>> + int low_irq, high_irq, ret = 0;
>>>> + unsigned long dft_low, dft_high;
>>>> +
>>>> + pzone = devm_kzalloc(&pdev->dev, sizeof(*pzone), GFP_KERNEL);
>>>> + if (!pzone)
>>>> + return -ENOMEM;
>>>> +
>>>> + ptrips = db8500_thermal_parse_dt(pdev);
>>>
>>> This is what u have in this routine at the very first line:
>>>
>>> if (!np) {
>>> dev_err(&pdev->dev, "Missing device tree data\n");
>>>
>>> So, you will end up printing this line for every non-DT case. Not good.
>>> What u can do is, give preference to normal pdata here.
>> I moved this if(!np) into parse_dt function, no problem again.
>> (in fact have already done this, but it is missed in this sending)
>
> Sorry couldn't get your point. :(
> Can you share diff of latest code in the same mail thread?
Just paste my current pieces of codes here:
static struct db8500_thsens_platform_data*
db8500_thermal_parse_dt(struct platform_device *pdev)
{
struct db8500_thsens_platform_data *ptrips;
struct device_node *np = pdev->dev.of_node;
char prop_name[32];
const char *tmp_str;
u32 tmp_data;
int i, j;
if (!np) {
dev_err(&pdev->dev, "Missing device tree data\n");
return NULL;
}
......
}
static int db8500_thermal_probe(struct platform_device *pdev)
{
struct db8500_thermal_zone *pzone = NULL;
struct db8500_thsens_platform_data *ptrips = NULL;
int low_irq, high_irq, ret = 0;
unsigned long dft_low, dft_high;
ptrips = db8500_thermal_parse_dt(pdev);
if (!ptrips)
ptrips = dev_get_platdata(&pdev->dev);
if (!ptrips)
return -EINVAL;
pzone = devm_kzalloc(&pdev->dev, sizeof(*pzone), GFP_KERNEL);
if (!pzone)
return -ENOMEM;
......
}
>
>>>> + ret = devm_request_threaded_irq(&pdev->dev, low_irq, NULL,
>>>
>>> why threaded irq?
>
>> In fact PRCMU firmware is polling the thermal sensor, and if it meets
>> threshold, the PRCMU will write this event into share memory (shared
>> between PRCMU and ARM) and trigger an interrupt to ARM.
>>
>> There may be other events passed via share memory, so it is better to
>> handle this kind of irq as fast as possible(it is always the policy),
>> and threaded irq satisfies this case better then the traditional one.
>
> Its been long that i prepared for an interview, but i believe purpose
> of threaded
> irq is something else.
>
> There can be two use cases of request_irq()
> - We don't want to sleep from interrupt handler, because we don't need to
> sleep
> for reading hardware's register. And so handler must be called from
> interrupt
> context. We use normal request_irq() here. This is the fastest one.
>
> - We have to sleep from some part of interrupt handler, because we don't have
> peripherals register on AMBA bus. But we have it on SPI or I2C bus,
> where read/
> write to SPI/I2C can potentially sleep. So, we want handler to execute from
> process context and so use request_threaded_irq(), i.e. handler will
> be called
> from a thread. This will surely be slow.
>
> Now in threaded irq case, we can give two handlers, one that must be called
> from interrupt context and other that must be called from process context.
> Both will be called one by one.
>
Understand your points.
> Sorry if i am wrong in my theory :(
> @Amit: Am i correct??
>
> Now, the same question again. Are you sure you want threaded irq here.
I just saw that all the PRCMU and ab8500 related irqs use request_threaded_irq
only difference is that I use devm_request_threaded_irq
>
>>>> + prcmu_low_irq_handler, IRQF_NO_SUSPEND | IRQF_ONESHOT,
>>>> + "dbx500_temp_low", pzone);
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
