[PATCH 1/1] power/supply/powersupply_sysfs: Add of_node name to uevent message if available

[PATCH 1/1] power/supply/powersupply_sysfs: Add of_node name to uevent message if available

[Richard Tresidder]

If the of_node name of the supply is available from the devicetree binding
then include it under the var POWER_SUPPLY_OF_NODE_NAME.
This helps where a consistent name is known via the device tree binding
but it is hard to identify based on the usual enumeration process.

Signed-off-by: Richard Tresidder <rtresidd@electromag.com.au>
---

Notes:
    power/supply/powersupply_sysfs: Add of_node name to uevent message if available
    
    If the of_node name of the supply is available from the devicetree binding
    then include it under the var POWER_SUPPLY_OF_NODE_NAME.
    This helps where a consistent name is known via the device tree binding
    but it is hard to identify based on the usual enumeration process.

 drivers/power/supply/power_supply_sysfs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index f37ad4e..ce6671c 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -395,6 +395,13 @@ int power_supply_uevent(struct device *dev, struct kobj_uevent_env *env)
 	if (ret)
 		return ret;
 
+	if (psy->of_node) {
+		ret = add_uevent_var(env, "POWER_SUPPLY_OF_NODE_NAME=%s",
+				     psy->of_node->name);
+		if (ret)
+			return ret;
+	}
+
 	prop_buf = (char *)get_zeroed_page(GFP_KERNEL);
 	if (!prop_buf)
 		return -ENOMEM;
-- 
1.8.3.1

Re: [PATCH 1/1] power/supply/powersupply_sysfs: Add of_node name to uevent message if available

[David Lechner]

On 7/25/19 2:47 AM, Richard Tresidder wrote:
> If the of_node name of the supply is available from the devicetree binding
> then include it under the var POWER_SUPPLY_OF_NODE_NAME.
> This helps where a consistent name is known via the device tree binding
> but it is hard to identify based on the usual enumeration process.
> 

Would it be possible to use of_device_uevent() instead of introducing a new
property?

Re: [PATCH 1/1] power/supply/powersupply_sysfs: Add of_node name to uevent message if available

[Richard Tresidder]

Hi David
   That it would be. I wasn't aware of that call.
I'll give it a crack and send in a new version of the patch.

Cheers
   Richard Tresidder

Richard Tresidder
On 25/07/2019 10:08 pm, David Lechner wrote:
> On 7/25/19 2:47 AM, Richard Tresidder wrote:
>> If the of_node name of the supply is available from the devicetree 
>> binding
>> then include it under the var POWER_SUPPLY_OF_NODE_NAME.
>> This helps where a consistent name is known via the device tree binding
>> but it is hard to identify based on the usual enumeration process.
>>
>
> Would it be possible to use of_device_uevent() instead of introducing 
> a new
> property?
>
>
>

Re: [PATCH 1/1] power/supply/powersupply_sysfs: Add of_node name to uevent message if available

[Richard Tresidder]

Hi David
   That call requires a struct device *
But the of_node pointer is located in the struct power_supply
Seems the of_node value in the base device* is null, and often is.
I find it hard at times to figure out which one of these to reference as 
things seem to be duplicated in a lot of places..

Should that call potentially be refactored to take a device_node * 
instead of a device *

**
Regards
    Richard Tresidder

On 26/07/2019 10:40 am, Richard Tresidder wrote:
> Hi David
>   That it would be. I wasn't aware of that call.
> I'll give it a crack and send in a new version of the patch.
>
> Cheers
>   Richard Tresidder
>
> Richard Tresidder
> On 25/07/2019 10:08 pm, David Lechner wrote:
>> On 7/25/19 2:47 AM, Richard Tresidder wrote:
>>> If the of_node name of the supply is available from the devicetree 
>>> binding
>>> then include it under the var POWER_SUPPLY_OF_NODE_NAME.
>>> This helps where a consistent name is known via the device tree binding
>>> but it is hard to identify based on the usual enumeration process.
>>>
>>
>> Would it be possible to use of_device_uevent() instead of introducing 
>> a new
>> property?
>>
>>
>>
>
>
>

Re: [PATCH 1/1] power/supply/powersupply_sysfs: Add of_node name to uevent message if available

[Richard Tresidder]

As per David's suggestion
I did a quick refactor of the of_device_uevent() call to handle a 
device_node "of_device_node_uevent()" instead of a device and ran into a 
problem.
Was getting stack dumps (via a WARN message) generated out of 
add_uevent_var() for any power supply device.
It was trigered in the first check of
     if (env->envp_idx >= ARRAY_SIZE(env->envp)) {
     ...
of that add_uevent_var function.
So it looks like to many value pairs were being shoved into the event?
envp array is set to a fixed length of 32 via the UEVENT_NUM_ENVP value 
in kobject.h

I verified it had nothing to do with with the refactor by backing it out 
and simply adding a few more
dummy add_uevent_var calls in the power_supply_uevent call..

so there appears to be a hard limit on this.. which we are very near for 
the power supply stuff.

Thoughts?

Thanks
    Richard Tresidder

On 26/07/2019 12:51 pm, Richard Tresidder wrote:
> Hi David
>   That call requires a struct device *
> But the of_node pointer is located in the struct power_supply
> Seems the of_node value in the base device* is null, and often is.
> I find it hard at times to figure out which one of these to reference 
> as things seem to be duplicated in a lot of places..
>
> Should that call potentially be refactored to take a device_node * 
> instead of a device *
>
> **
> Regards
>    Richard Tresidder
>
> On 26/07/2019 10:40 am, Richard Tresidder wrote:
>> Hi David
>>   That it would be. I wasn't aware of that call.
>> I'll give it a crack and send in a new version of the patch.
>>
>> Cheers
>>   Richard Tresidder
>>
>> Richard Tresidder
>> On 25/07/2019 10:08 pm, David Lechner wrote:
>>> On 7/25/19 2:47 AM, Richard Tresidder wrote:
>>>> If the of_node name of the supply is available from the devicetree 
>>>> binding
>>>> then include it under the var POWER_SUPPLY_OF_NODE_NAME.
>>>> This helps where a consistent name is known via the device tree 
>>>> binding
>>>> but it is hard to identify based on the usual enumeration process.
>>>>
>>>
>>> Would it be possible to use of_device_uevent() instead of 
>>> introducing a new
>>> property?
>>>
>>>
>>>
>>
>>
>>
>
>
>

Re: [PATCH 1/1] power/supply/powersupply_sysfs: Add of_node name to uevent message if available

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1992 bytes --]

Hi Richard,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.3-rc1 next-20190726]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Richard-Tresidder/power-supply-powersupply_sysfs-Add-of_node-name-to-uevent-message-if-available/20190727-033750
config: arm-omap2plus_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/power/supply/power_supply_sysfs.c: In function 'power_supply_uevent':
>> drivers/power/supply/power_supply_sysfs.c:400:22: error: dereferencing pointer to incomplete type 'struct device_node'
             psy->of_node->name);
                         ^~

vim +400 drivers/power/supply/power_supply_sysfs.c

   381	
   382	int power_supply_uevent(struct device *dev, struct kobj_uevent_env *env)
   383	{
   384		struct power_supply *psy = dev_get_drvdata(dev);
   385		int ret = 0, j;
   386		char *prop_buf;
   387		char *attrname;
   388	
   389		if (!psy || !psy->desc) {
   390			dev_dbg(dev, "No power supply yet\n");
   391			return ret;
   392		}
   393	
   394		ret = add_uevent_var(env, "POWER_SUPPLY_NAME=%s", psy->desc->name);
   395		if (ret)
   396			return ret;
   397	
   398		if (psy->of_node) {
   399			ret = add_uevent_var(env, "POWER_SUPPLY_OF_NODE_NAME=%s",
 > 400					     psy->of_node->name);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36719 bytes --]

Re: [PATCH 1/1] power/supply/powersupply_sysfs: Add of_node name to uevent message if available

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1201 bytes --]

Hi Richard,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc2 next-20190729]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Richard-Tresidder/power-supply-powersupply_sysfs-Add-of_node-name-to-uevent-message-if-available/20190727-033750
config: x86_64-randconfig-s0-07280001 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.2-10+deb8u1) 4.9.2
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/power/supply/power_supply_sysfs.c: In function 'power_supply_uevent':
>> drivers/power/supply/power_supply_sysfs.c:400:22: error: dereferencing pointer to incomplete type
             psy->of_node->name);
                         ^

vim +400 drivers/power/supply/power_supply_sysfs.c

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33734 bytes --]