[PATCH] thermal: of: look for sensor driver parent node if device node missing

[PATCH] thermal: of: look for sensor driver parent node if device node missing

[Laxman Dewangan]

There are some mfd devices which supports junction thermal interrupt
like ams,AS3722. The DT binding of these devices are defined as the
flat and drivers for sub module of such devices are registered as
the mfd_add_devices. In this method, the sub devices registered as
platform driver and these do not have the of_node pointer on their
device structure. In this case, use the parent of_node pointer to
get the required of_node pointer.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/thermal/of-thermal.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 04b1be7..85a7d71 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -396,6 +396,8 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id,
 		return ERR_PTR(-EINVAL);
 
 	sensor_np = dev->of_node;
+	if (!sensor_np && dev->parent)
+		sensor_np = dev->parent->of_node;
 
 	for_each_child_of_node(np, child) {
 		struct of_phandle_args sensor_specs;
-- 
1.8.1.5

[PATCH] thermal: of: look for sensor driver parent node if device node missing

[Laxman Dewangan]

There are some mfd devices which supports junction thermal interrupt
like ams,AS3722. The DT binding of these devices are defined as the
flat and drivers for sub module of such devices are registered as
the mfd_add_devices. In this method, the sub devices registered as
platform driver and these do not have the of_node pointer on their
device structure. In this case, use the parent of_node pointer to
get the required of_node pointer.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/thermal/of-thermal.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 04b1be7..85a7d71 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -396,6 +396,8 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id,
 		return ERR_PTR(-EINVAL);
 
 	sensor_np = dev->of_node;
+	if (!sensor_np && dev->parent)
+		sensor_np = dev->parent->of_node;
 
 	for_each_child_of_node(np, child) {
 		struct of_phandle_args sensor_specs;
-- 
1.8.1.5

Re: [PATCH] thermal: of: look for sensor driver parent node if device node missing

[Lucas Stach]

Am Montag, den 14.07.2014, 16:42 +0530 schrieb Laxman Dewangan:
> There are some mfd devices which supports junction thermal interrupt
> like ams,AS3722. The DT binding of these devices are defined as the
> flat and drivers for sub module of such devices are registered as
> the mfd_add_devices. In this method, the sub devices registered as
> platform driver and these do not have the of_node pointer on their
> device structure. In this case, use the parent of_node pointer to
> get the required of_node pointer.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
>  drivers/thermal/of-thermal.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index 04b1be7..85a7d71 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -396,6 +396,8 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id,
>  		return ERR_PTR(-EINVAL);
>  
>  	sensor_np = dev->of_node;
> +	if (!sensor_np && dev->parent)
> +		sensor_np = dev->parent->of_node;
>  
>  	for_each_child_of_node(np, child) {
>  		struct of_phandle_args sensor_specs;

This seems like the wrong way around. If the MFD has subdev information
stored in the parent node it should be the MFD drivers responsibility to
populate the subdev of_node with its own node. The subdev should not be
forced to make such possible unsafe assumptions.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [PATCH] thermal: of: look for sensor driver parent node if device node missing

[Laxman Dewangan]

On Monday 14 July 2014 04:42 PM, Laxman Dewangan wrote:
> There are some mfd devices which supports junction thermal interrupt
> like ams,AS3722. The DT binding of these devices are defined as the
> flat and drivers for sub module of such devices are registered as
> the mfd_add_devices. In this method, the sub devices registered as
> platform driver and these do not have the of_node pointer on their
> device structure. In this case, use the parent of_node pointer to
> get the required of_node pointer.
>
Any comment please?

RE: [PATCH] thermal: of: look for sensor driver parent node if device node missing

[Zhang, Rui]

Hi, Laxman,

As Eduardo is the of thermal author and maintainer, I will take your patch only if you can get ACK from Eduardo.

Eduardo,
Do you have any comments on this?

Thanks,
rui

> -----Original Message-----
> From: Laxman Dewangan [mailto:ldewangan@nvidia.com]
> Sent: Thursday, July 24, 2014 5:49 PM
> To: Zhang, Rui; eduardo.valentin@ti.com
> Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] thermal: of: look for sensor driver parent node if
> device node missing
> Importance: High
> 
> On Monday 14 July 2014 04:42 PM, Laxman Dewangan wrote:
> > There are some mfd devices which supports junction thermal interrupt
> > like ams,AS3722. The DT binding of these devices are defined as the
> > flat and drivers for sub module of such devices are registered as the
> > mfd_add_devices. In this method, the sub devices registered as
> > platform driver and these do not have the of_node pointer on their
> > device structure. In this case, use the parent of_node pointer to get
> > the required of_node pointer.
> >
> Any comment please?

Re: [PATCH] thermal: of: look for sensor driver parent node if device node missing

[Eduardo Valentin]

Hello,

Perhaps you may have sent this email to the wrong Eduardo as I do not recognize the topic you are discussing.

Regards,

Sent from my iPhone

> On Jul 24, 2014, at 11:15 AM, "Zhang, Rui" <rui.zhang@intel.com> wrote:
> 
> Hi, Laxman,
> 
> As Eduardo is the of thermal author and maintainer, I will take your patch only if you can get ACK from Eduardo.
> 
> Eduardo,
> Do you have any comments on this?
> 
> Thanks,
> rui
> 
>> -----Original Message-----
>> From: Laxman Dewangan [mailto:ldewangan@nvidia.com]
>> Sent: Thursday, July 24, 2014 5:49 PM
>> To: Zhang, Rui; eduardo.valentin@ti.com
>> Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH] thermal: of: look for sensor driver parent node if
>> device node missing
>> Importance: High
>> 
>>> On Monday 14 July 2014 04:42 PM, Laxman Dewangan wrote:
>>> There are some mfd devices which supports junction thermal interrupt
>>> like ams,AS3722. The DT binding of these devices are defined as the
>>> flat and drivers for sub module of such devices are registered as the
>>> mfd_add_devices. In this method, the sub devices registered as
>>> platform driver and these do not have the of_node pointer on their
>>> device structure. In this case, use the parent of_node pointer to get
>>> the required of_node pointer.
>> Any comment please?

Re: [PATCH] thermal: of: look for sensor driver parent node if device node missing

[Laxman Dewangan]

Thanks Rui.
It seems I have put the wrong email-id for Eduardo (which I got from 
get_maintainer) and the original patch not reached to Eduardo.

Do I need to re-post patch?

Thanks,
Laxman

On Thursday 24 July 2014 08:45 PM, Zhang, Rui wrote:
> Hi, Laxman,
>
> As Eduardo is the of thermal author and maintainer, I will take your patch only if you can get ACK from Eduardo.
>
> Eduardo,
> Do you have any comments on this?
>
> Thanks,
> rui
>
>> -----Original Message-----
>> From: Laxman Dewangan [mailto:ldewangan@nvidia.com]
>> Sent: Thursday, July 24, 2014 5:49 PM
>> To: Zhang, Rui; eduardo.valentin@ti.com
>> Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH] thermal: of: look for sensor driver parent node if
>> device node missing
>> Importance: High
>>
>> On Monday 14 July 2014 04:42 PM, Laxman Dewangan wrote:
>>> There are some mfd devices which supports junction thermal interrupt
>>> like ams,AS3722. The DT binding of these devices are defined as the
>>> flat and drivers for sub module of such devices are registered as the
>>> mfd_add_devices. In this method, the sub devices registered as
>>> platform driver and these do not have the of_node pointer on their
>>> device structure. In this case, use the parent of_node pointer to get
>>> the required of_node pointer.
>>>
>> Any comment please?

Re: [PATCH] thermal: of: look for sensor driver parent node if device node missing

[Javi Merino]

On Fri, Jul 25, 2014 at 09:22:00AM +0100, Laxman Dewangan wrote:
> Thanks Rui.
> It seems I have put the wrong email-id for Eduardo (which I got from 
> get_maintainer) and the original patch not reached to Eduardo.
> 
> Do I need to re-post patch?
> 
> Thanks,
> Laxman
> 
> On Thursday 24 July 2014 08:45 PM, Zhang, Rui wrote:
> > Hi, Laxman,
> >
> > As Eduardo is the of thermal author and maintainer, I will take your patch only if you can get ACK from Eduardo.
> >
> > Eduardo,
> > Do you have any comments on this?

[Fixed Eduardo's email.]

> >> -----Original Message-----
> >> From: Laxman Dewangan [mailto:ldewangan@nvidia.com]
> >> Sent: Thursday, July 24, 2014 5:49 PM
> >> To: Zhang, Rui; eduardo.valentin@ti.com
> >> Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> >> Subject: Re: [PATCH] thermal: of: look for sensor driver parent node if
> >> device node missing
> >> Importance: High
> >>
> >> On Monday 14 July 2014 04:42 PM, Laxman Dewangan wrote:
> >>> There are some mfd devices which supports junction thermal interrupt
> >>> like ams,AS3722. The DT binding of these devices are defined as the
> >>> flat and drivers for sub module of such devices are registered as the
> >>> mfd_add_devices. In this method, the sub devices registered as
> >>> platform driver and these do not have the of_node pointer on their
> >>> device structure. In this case, use the parent of_node pointer to get
> >>> the required of_node pointer.
> >>>
> >> Any comment please?
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] thermal: of: look for sensor driver parent node if device node missing

[Eduardo Valentin]

Laxman,

On Mon, Jul 14, 2014 at 02:04:55PM +0200, Lucas Stach wrote:
> Am Montag, den 14.07.2014, 16:42 +0530 schrieb Laxman Dewangan:
> > There are some mfd devices which supports junction thermal interrupt
> > like ams,AS3722. The DT binding of these devices are defined as the
> > flat and drivers for sub module of such devices are registered as
> > the mfd_add_devices. In this method, the sub devices registered as
> > platform driver and these do not have the of_node pointer on their
> > device structure. In this case, use the parent of_node pointer to
> > get the required of_node pointer.
> > 
> > Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> > ---
> >  drivers/thermal/of-thermal.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> > index 04b1be7..85a7d71 100644
> > --- a/drivers/thermal/of-thermal.c
> > +++ b/drivers/thermal/of-thermal.c
> > @@ -396,6 +396,8 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id,
> >  		return ERR_PTR(-EINVAL);
> >  
> >  	sensor_np = dev->of_node;
> > +	if (!sensor_np && dev->parent)
> > +		sensor_np = dev->parent->of_node;
> >  
> >  	for_each_child_of_node(np, child) {
> >  		struct of_phandle_args sensor_specs;
> 
> This seems like the wrong way around. If the MFD has subdev information
> stored in the parent node it should be the MFD drivers responsibility to
> populate the subdev of_node with its own node. The subdev should not be
> forced to make such possible unsafe assumptions.
> 

This is my understanding too. MFD device drivers must populate the
required data onto their sub devices while creating them.

> Regards,
> Lucas
> -- 
> Pengutronix e.K.             | Lucas Stach                 |
> Industrial Linux Solutions   | http://www.pengutronix.de/  |
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] thermal: of: look for sensor driver parent node if device node missing

[Laxman Dewangan]

On Wednesday 30 July 2014 06:48 PM, Eduardo Valentin wrote:
> Laxman,
>
> On Mon, Jul 14, 2014 at 02:04:55PM +0200, Lucas Stach wrote:
>> Am Montag, den 14.07.2014, 16:42 +0530 schrieb Laxman Dewangan:
>>> There are some mfd devices which supports junction thermal interrupt
>>> like ams,AS3722. The DT binding of these devices are defined as the
>>> flat and drivers for sub module of such devices are registered as
>>> the mfd_add_devices. In this method, the sub devices registered as
>>> platform driver and these do not have the of_node pointer on their
>>> device structure. In this case, use the parent of_node pointer to
>>> get the required of_node pointer.
>>>
>>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>>> ---
>>>   drivers/thermal/of-thermal.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
>>> index 04b1be7..85a7d71 100644
>>> --- a/drivers/thermal/of-thermal.c
>>> +++ b/drivers/thermal/of-thermal.c
>>> @@ -396,6 +396,8 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id,
>>>   		return ERR_PTR(-EINVAL);
>>>   
>>>   	sensor_np = dev->of_node;
>>> +	if (!sensor_np && dev->parent)
>>> +		sensor_np = dev->parent->of_node;
>>>   
>>>   	for_each_child_of_node(np, child) {
>>>   		struct of_phandle_args sensor_specs;
>> This seems like the wrong way around. If the MFD has subdev information
>> stored in the parent node it should be the MFD drivers responsibility to
>> populate the subdev of_node with its own node. The subdev should not be
>> forced to make such possible unsafe assumptions.
>>
> This is my understanding too. MFD device drivers must populate the
> required data onto their sub devices while creating them.
>

Sometime ago, Stephen sent patch on MFD to initialize the mfd sub 
devices of_node with parent node.

mfd: always assign of_node in mfd_add_device()
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-December/217482.html

But it has unforeseen issue and reverted with patch
mfd: Revert "mfd: Always assign of_node in mfd_add_device()"
https://groups.google.com/forum/#!msg/rtc-linux/2LeLipSPFs0/XfPCKpT1BnUJ

So we do not have any mechanism here to pass the parent of_node to the 
of-thermal.

Adding Stephen and Lee for more comment.

Re: [PATCH] thermal: of: look for sensor driver parent node if device node missing

[Eduardo Valentin]

Hello Laxman,

On Wed, Jul 30, 2014 at 07:09:04PM +0530, Laxman Dewangan wrote:
> On Wednesday 30 July 2014 06:48 PM, Eduardo Valentin wrote:
> > Laxman,
> >
> > On Mon, Jul 14, 2014 at 02:04:55PM +0200, Lucas Stach wrote:
> >> Am Montag, den 14.07.2014, 16:42 +0530 schrieb Laxman Dewangan:
> >>> There are some mfd devices which supports junction thermal interrupt
> >>> like ams,AS3722. The DT binding of these devices are defined as the
> >>> flat and drivers for sub module of such devices are registered as
> >>> the mfd_add_devices. In this method, the sub devices registered as
> >>> platform driver and these do not have the of_node pointer on their
> >>> device structure. In this case, use the parent of_node pointer to
> >>> get the required of_node pointer.
> >>>
> >>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> >>> ---
> >>>   drivers/thermal/of-thermal.c | 2 ++
> >>>   1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> >>> index 04b1be7..85a7d71 100644
> >>> --- a/drivers/thermal/of-thermal.c
> >>> +++ b/drivers/thermal/of-thermal.c
> >>> @@ -396,6 +396,8 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id,
> >>>   		return ERR_PTR(-EINVAL);
> >>>   
> >>>   	sensor_np = dev->of_node;
> >>> +	if (!sensor_np && dev->parent)
> >>> +		sensor_np = dev->parent->of_node;
> >>>   
> >>>   	for_each_child_of_node(np, child) {
> >>>   		struct of_phandle_args sensor_specs;
> >> This seems like the wrong way around. If the MFD has subdev information
> >> stored in the parent node it should be the MFD drivers responsibility to
> >> populate the subdev of_node with its own node. The subdev should not be
> >> forced to make such possible unsafe assumptions.
> >>
> > This is my understanding too. MFD device drivers must populate the
> > required data onto their sub devices while creating them.
> >
> 
> Sometime ago, Stephen sent patch on MFD to initialize the mfd sub 
> devices of_node with parent node.
> 
> mfd: always assign of_node in mfd_add_device()
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-December/217482.html
> 

In fact, as I mentioned, this is a controversal subject.

> But it has unforeseen issue and reverted with patch
> mfd: Revert "mfd: Always assign of_node in mfd_add_device()"
> https://groups.google.com/forum/#!msg/rtc-linux/2LeLipSPFs0/XfPCKpT1BnUJ
> 

Indeed, the strategy had issues.

> So we do not have any mechanism here to pass the parent of_node to the 
> of-thermal.

Yeah, but still, as you can see this issue is present in other places apart from
of-thermal. Therefore, it is clear that needs to be fixed somewhere else
than in of-thermal so that others benefit too, don't you agree?

> 
> Adding Stephen and Lee for more comment.

Sure.

In general, I believe letting sub devices to access the MFD core data
(in this case, the parent of node data) is somehow racy. I would prefer
we could have a way to populate the of_node of subdevs with only the
required info.