Re: [PATCH] stmmac: debugfs entry name is not be changed when udev rename device name.

Re: [PATCH] stmmac: debugfs entry name is not be changed when udev rename device name.

[Randy Dunlap]

Hi,

On 12/30/19 6:03 PM, Jiping Ma wrote:
> Add one notifier for udev changes net device name.
> 
> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
> ---
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 38 ++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index b14f46a57154..c1c877bb4421 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4038,6 +4038,40 @@ static int stmmac_dma_cap_show(struct seq_file *seq, void *v)
>  }
>  DEFINE_SHOW_ATTRIBUTE(stmmac_dma_cap);
>  
> +/**

Just use /* here since this is not a kernel-doc comment.
/** is reserved for kernel-doc comments/notation.

> + * Use network device events to create/remove/rename
> + * debugfs file entries
> + */
> +static int stmmac_device_event(struct notifier_block *unused,
> +			       unsigned long event, void *ptr)
> +{


> @@ -4050,7 +4084,6 @@ static int stmmac_init_fs(struct net_device *dev)
>  
>  		return -ENOMEM;
>  	}
> -
>  	/* Entry to report DMA RX/TX rings */
>  	priv->dbgfs_rings_status =
>  		debugfs_create_file("descriptors_status", 0444,

I don't see a problem with the blank line.  Why remove it?


thanks.
-- 
~Randy


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] stmmac: debugfs entry name is not be changed when udev rename device name.

[Randy Dunlap]

On 12/30/19 6:16 PM, Jiping Ma wrote:
> 
> 
> On 12/31/2019 10:11 AM, Randy Dunlap wrote:
>> Hi,
>>
>> On 12/30/19 6:03 PM, Jiping Ma wrote:
>>> Add one notifier for udev changes net device name.
>>>
>>> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
>>> ---
>>>   .../net/ethernet/stmicro/stmmac/stmmac_main.c | 38 ++++++++++++++++++-
>>>   1 file changed, 37 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> index b14f46a57154..c1c877bb4421 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> @@ -4038,6 +4038,40 @@ static int stmmac_dma_cap_show(struct seq_file *seq, void *v)
>>>   }
>>>   DEFINE_SHOW_ATTRIBUTE(stmmac_dma_cap);
>>>   +/**
>> Just use /* here since this is not a kernel-doc comment.
>> /** is reserved for kernel-doc comments/notation.
> I use checkpatch.pl to check my patch, it show one warning, then I change * to **.   I will change it back to *.

It should be more like:

/* Use network device events to create/remove/rename
 * debugfs file entries.
 */

> WARNING: networking block comments don't use an empty /* line, use /* Comment...
> #23: FILE: drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:4042:
> +/*
> + * Use network device events to create/remove/rename
>>
>>> + * Use network device events to create/remove/rename
>>> + * debugfs file entries
>>> + */
>>> +static int stmmac_device_event(struct notifier_block *unused,
>>> +                   unsigned long event, void *ptr)
>>> +{
>>


-- 
~Randy
Reported-by: Randy Dunlap <rdunlap@infradead.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] stmmac: debugfs entry name is not be changed when udev rename device name.

[Sergei Shtylyov]

Hello!

On 31.12.2019 5:03, Jiping Ma wrote:

> Add one notifier for udev changes net device name.
> 
> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
> ---
>   .../net/ethernet/stmicro/stmmac/stmmac_main.c | 38 ++++++++++++++++++-
>   1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index b14f46a57154..c1c877bb4421 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4038,6 +4038,40 @@ static int stmmac_dma_cap_show(struct seq_file *seq, void *v)
>   }
>   DEFINE_SHOW_ATTRIBUTE(stmmac_dma_cap);
>   
> +/**
> + * Use network device events to create/remove/rename
> + * debugfs file entries
> + */
> +static int stmmac_device_event(struct notifier_block *unused,
> +			       unsigned long event, void *ptr)
> +{
> +	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
> +	struct stmmac_priv *priv = netdev_priv(dev);
> +
> +	switch (event) {
> +	case NETDEV_CHANGENAME:
> +		if (priv->dbgfs_dir)
> +			priv->dbgfs_dir = debugfs_rename(stmmac_fs_dir,
> +							 priv->dbgfs_dir,
> +							 stmmac_fs_dir,
> +							 dev->name);
> +		break;
> +
> +	case NETDEV_GOING_DOWN:
> +		break;
> +
> +	case NETDEV_UP:
> +		break;

    Why not merge the above 2 cases? Or just remove them('event' is not *enum*)?

> +	}
> +
> +done:
> +	return NOTIFY_DONE;
> +}
[...]

MBR, Sergei

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] stmmac: debugfs entry name is not be changed when udev rename device name.

[David Miller]

From: Jiping Ma <Jiping.Ma2@windriver.com>
Date: Tue, 7 Jan 2020 10:59:22 +0800

> 
> 
> On 01/07/2020 10:22 AM, David Miller wrote:
>> From: Jiping Ma <Jiping.Ma2@windriver.com>
>> Date: Tue, 7 Jan 2020 09:00:53 +0800
>>
>>>
>>> On 01/07/2020 05:45 AM, David Miller wrote:
>>>> From: Jiping Ma <jiping.ma2@windriver.com>
>>>> Date: Mon, 6 Jan 2020 10:33:41 +0800
>>>>
>>>>> Add one notifier for udev changes net device name.
>>>>>
>>>>> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
>>>> This doesn't apply to 'net' and since this is a bug fix that is where
>>>> you should target this change.
>>> What's the next step that I can do?
>> Respin your patch against the net GIT tree so that it applies clean.y
> OK, I will generate the new patch based on the latest linux kernel
> code.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

That's not the networking GIT tree.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] stmmac: debugfs entry name is not be changed when udev rename device name.

[Florian Fainelli]

On 1/6/2020 6:59 PM, Jiping Ma wrote:
> 
> 
> On 01/07/2020 10:22 AM, David Miller wrote:
>> From: Jiping Ma <Jiping.Ma2@windriver.com>
>> Date: Tue, 7 Jan 2020 09:00:53 +0800
>>
>>>
>>> On 01/07/2020 05:45 AM, David Miller wrote:
>>>> From: Jiping Ma <jiping.ma2@windriver.com>
>>>> Date: Mon, 6 Jan 2020 10:33:41 +0800
>>>>
>>>>> Add one notifier for udev changes net device name.
>>>>>
>>>>> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
>>>> This doesn't apply to 'net' and since this is a bug fix that is where
>>>> you should target this change.
>>> What's the next step that I can do?
>> Respin your patch against the net GIT tree so that it applies clean.y
> OK, I will generate the new patch based on the latest linux kernel code.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

That is not quite the tree you should be using, you should be using the
"net" tree, which is here:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/

and here are some useful pieces of information in the netdev FAQ:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/Documentation/networking/netdev-FAQ.rst#n28

Also, since this is not the version version of your patch, please make
sure your subject indicates this is not the first version, I have lost
count of the exact number of times you submitted this already (4?) so
this could be [PATCH net v5] in the subject or something like that.

Given you fix a bug please also provide an appropriate Fixes tag, which
should be:

Fixes: 466c5ac8bdf2 ("net: stmmac: create one debugfs dir per net-device")
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] stmmac: debugfs entry name is not be changed when udev rename device name.

[David Miller]

From: Jiping Ma <Jiping.Ma2@windriver.com>
Date: Tue, 7 Jan 2020 09:00:53 +0800

> 
> 
> On 01/07/2020 05:45 AM, David Miller wrote:
>> From: Jiping Ma <jiping.ma2@windriver.com>
>> Date: Mon, 6 Jan 2020 10:33:41 +0800
>>
>>> Add one notifier for udev changes net device name.
>>>
>>> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
>> This doesn't apply to 'net' and since this is a bug fix that is where
>> you should target this change.
> What's the next step that I can do?

Respin your patch against the net GIT tree so that it applies clean.y

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] stmmac: debugfs entry name is not be changed when udev rename device name.

[David Miller]

From: Jiping Ma <jiping.ma2@windriver.com>
Date: Mon, 6 Jan 2020 10:33:41 +0800

> Add one notifier for udev changes net device name.
> 
> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>

This doesn't apply to 'net' and since this is a bug fix that is where
you should target this change.

Thank you.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] stmmac: debugfs entry name is not be changed when udev rename device name.

[David Miller]

From: Jiping Ma <jiping.ma2@windriver.com>
Date: Thu, 2 Jan 2020 09:35:44 +0800

> Add one notifier for udev changes net device name.
> 
> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
> ---
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index b14f46a57154..3b05cb80eed7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4038,6 +4038,31 @@ static int stmmac_dma_cap_show(struct seq_file *seq, void *v)
>  }
>  DEFINE_SHOW_ATTRIBUTE(stmmac_dma_cap);
>  
> +/* Use network device events to rename debugfs file entries.
> + */
> +static int stmmac_device_event(struct notifier_block *unused,
> +			       unsigned long event, void *ptr)
> +{
> +	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
> +	struct stmmac_priv *priv = netdev_priv(dev);
> +
> +	switch (event) {
> +	case NETDEV_CHANGENAME:

This notifier gets called for every single netdevice in the entire
system.

You cannot just assume that the device that gets passed in here is
an stmmac device.

Look at how other drivers handle this to see how to do it correctly.

Thank you.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel