drivers/net/ethernet/sunplus/spl2sw_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
ndo_do_ioctl is no longer called by the device ioctl handler,
so use ndo_eth_ioctl instead.
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
drivers/net/ethernet/sunplus/spl2sw_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/sunplus/spl2sw_driver.c b/drivers/net/ethernet/sunplus/spl2sw_driver.c
index 721d8ed3f302..5e0e4c9ecbb0 100644
--- a/drivers/net/ethernet/sunplus/spl2sw_driver.c
+++ b/drivers/net/ethernet/sunplus/spl2sw_driver.c
@@ -199,7 +199,7 @@ static const struct net_device_ops netdev_ops = {
.ndo_start_xmit = spl2sw_ethernet_start_xmit,
.ndo_set_rx_mode = spl2sw_ethernet_set_rx_mode,
.ndo_set_mac_address = spl2sw_ethernet_set_mac_address,
- .ndo_do_ioctl = phy_do_ioctl,
+ .ndo_eth_ioctl = phy_do_ioctl,
.ndo_tx_timeout = spl2sw_ethernet_tx_timeout,
};
--
2.43.0
On Wed, 8 Jan 2025 14:12:38 +0000 Yeking@Red54.com wrote: > From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com> > > ndo_do_ioctl is no longer called by the device ioctl handler, > so use ndo_eth_ioctl instead. I presume this used to work and now it doesn't? If so a Fixes tag pointing to the commit that broke it would be in order. Please also mention how you tested this. Is this something you actually run into on real HW? Or just found by code inspection and only compile tested? -- pw-bot: cr
From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
ndo_do_ioctl is no longer called by the device ioctl handler,
so use ndo_eth_ioctl instead. (found by code inspection)
Fixes: fd3040b9394c ("net: ethernet: Add driver for Sunplus SP7021")
Fixes: a76053707dbf ("dev_ioctl: split out ndo_eth_ioctl")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
V1 -> V2: Update commit message
drivers/net/ethernet/sunplus/spl2sw_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/sunplus/spl2sw_driver.c b/drivers/net/ethernet/sunplus/spl2sw_driver.c
index 721d8ed3f302..5e0e4c9ecbb0 100644
--- a/drivers/net/ethernet/sunplus/spl2sw_driver.c
+++ b/drivers/net/ethernet/sunplus/spl2sw_driver.c
@@ -199,7 +199,7 @@ static const struct net_device_ops netdev_ops = {
.ndo_start_xmit = spl2sw_ethernet_start_xmit,
.ndo_set_rx_mode = spl2sw_ethernet_set_rx_mode,
.ndo_set_mac_address = spl2sw_ethernet_set_mac_address,
- .ndo_do_ioctl = phy_do_ioctl,
+ .ndo_eth_ioctl = phy_do_ioctl,
.ndo_tx_timeout = spl2sw_ethernet_tx_timeout,
};
--
2.43.0
On Thu, 9 Jan 2025 02:05:52 +0000 Yeking@Red54.com wrote:
> From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
>
> ndo_do_ioctl is no longer called by the device ioctl handler,
> so use ndo_eth_ioctl instead. (found by code inspection)
>
> Fixes: fd3040b9394c ("net: ethernet: Add driver for Sunplus SP7021")
This tag is unnecessary, Fixes tag should point to the commit which
_broke_ things.
> Fixes: a76053707dbf ("dev_ioctl: split out ndo_eth_ioctl")
Now that you added this tag you need to run get_maintainer again /
correctly on the patch and CC the authors.
--
pw-bot: cr
From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
The device ioctl handler no longer calls ndo_do_ioctl, but calls
ndo_eth_ioctl to handle mii ioctls. However, sunplus still used
ndo_do_ioctl when it was introduced. So switch to ndo_eth_ioctl. (found
by code inspection)
Fixes: fd3040b9394c ("net: ethernet: Add driver for Sunplus SP7021", 2022-05-08)
Fixes: a76053707dbf ("dev_ioctl: split out ndo_eth_ioctl", 2021-07-27)
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
V2 -> V3: Update commit message again, and add short author date to the
Fixes tag to make it clear at a glance and reduce misunderstandings.
V1 -> V2: Update commit message
To Jakub Kicinski:
The old Fixes tag style is at least 10 years old. It lacks date
information, which can lead to misjudgment. So I added short author date
to avoid this. However, if you disagree, I can change it back to the old
Fixes tag style.
drivers/net/ethernet/sunplus/spl2sw_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/sunplus/spl2sw_driver.c b/drivers/net/ethernet/sunplus/spl2sw_driver.c
index 721d8ed3f302..5e0e4c9ecbb0 100644
--- a/drivers/net/ethernet/sunplus/spl2sw_driver.c
+++ b/drivers/net/ethernet/sunplus/spl2sw_driver.c
@@ -199,7 +199,7 @@ static const struct net_device_ops netdev_ops = {
.ndo_start_xmit = spl2sw_ethernet_start_xmit,
.ndo_set_rx_mode = spl2sw_ethernet_set_rx_mode,
.ndo_set_mac_address = spl2sw_ethernet_set_mac_address,
- .ndo_do_ioctl = phy_do_ioctl,
+ .ndo_eth_ioctl = phy_do_ioctl,
.ndo_tx_timeout = spl2sw_ethernet_tx_timeout,
};
--
2.43.0
From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
The old Fixes tag style is at least 10 years old. It lacks date
information, which can lead to misjudgment. So I added short author date
to avoid this. This make it clear at a glance and reduce
misunderstandings.
For example:
Old Fixes tag style:
* Fixes: fd3040b9394c ("net: ethernet: Add driver for Sunplus SP7021")
* Fixes: a76053707dbf ("dev_ioctl: split out ndo_eth_ioctl")
This will make people mistakenly think that "a76053707dbf" broke
"fd3040b9394c".[1]
New Fixes tag style:
* Fixes: fd3040b9394c ("net: ethernet: Add driver for Sunplus SP7021", 2022-05-08)
* Fixes: a76053707dbf ("dev_ioctl: split out ndo_eth_ioctl", 2021-07-27)
This makes it clear that the newly introduced driver did not follow the
existing changes.
[1] https://lore.kernel.org/all/20250109180212.71e4e53c@kernel.org/
docs: submitting-patches: The short author date of old example
"54a4f0239f2e" is 2010-05-05, which is not immediately obvious as
YYYY-MM-DD, so change example.
Fixes: 8401aa1f5997 ("Documentation/SubmittingPatches: describe the Fixes: tag", 2014-06-06)
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
Documentation/process/5.Posting.rst | 2 +-
Documentation/process/maintainer-tip.rst | 4 +--
.../process/researcher-guidelines.rst | 2 +-
Documentation/process/submitting-patches.rst | 8 ++---
scripts/checkpatch.pl | 30 +++++++++++--------
5 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/Documentation/process/5.Posting.rst b/Documentation/process/5.Posting.rst
index b3eff03ea249..38c9c94e7448 100644
--- a/Documentation/process/5.Posting.rst
+++ b/Documentation/process/5.Posting.rst
@@ -199,7 +199,7 @@ document; what follows here is a brief summary.
One tag is used to refer to earlier commits which introduced problems fixed by
the patch::
- Fixes: 1f2e3d4c5b6a ("The first line of the commit specified by the first 12 characters of its SHA-1 ID")
+ Fixes: 1f2e3d4c5b6a ("The first line of the commit specified by the first 12 characters of its SHA-1 ID", 2024-12-31)
Another tag is used for linking web pages with additional backgrounds or
details, for example an earlier discussion which leads to the patch or a
diff --git a/Documentation/process/maintainer-tip.rst b/Documentation/process/maintainer-tip.rst
index e374b67b3277..fb97a6853e42 100644
--- a/Documentation/process/maintainer-tip.rst
+++ b/Documentation/process/maintainer-tip.rst
@@ -270,7 +270,7 @@ Ordering of commit tags
To have a uniform view of the commit tags, the tip maintainers use the
following tag ordering scheme:
- - Fixes: 12char-SHA1 ("sub/sys: Original subject line")
+ - Fixes: 12char-SHA1 ("sub/sys: Original subject line", YYYY-MM-DD)
A Fixes tag should be added even for changes which do not need to be
backported to stable kernels, i.e. when addressing a recently introduced
@@ -295,7 +295,7 @@ following tag ordering scheme:
The recent replacement of foo with bar left an unused instance of
variable foo around. Remove it.
- Fixes: abcdef012345678 ("x86/xxx: Replace foo with bar")
+ Fixes: abcdef012345678 ("x86/xxx: Replace foo with bar", 2024-12-31)
Signed-off-by: J.Dev <j.dev@mail>
The latter puts the information about the patch into the focus and
diff --git a/Documentation/process/researcher-guidelines.rst b/Documentation/process/researcher-guidelines.rst
index beb484c5965d..472ca0a4684d 100644
--- a/Documentation/process/researcher-guidelines.rst
+++ b/Documentation/process/researcher-guidelines.rst
@@ -149,7 +149,7 @@ For example::
[1] https://url/to/leakmagic/details
Reported-by: Researcher <researcher@email>
- Fixes: aaaabbbbccccdddd ("Introduce support for FooBar")
+ Fixes: aaaabbbbccccdddd ("Introduce support for FooBar", 2024-12-31)
Signed-off-by: Author <author@email>
Reviewed-by: Reviewer <reviewer@email>
diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index 1518bd57adab..4df1e722130b 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -148,7 +148,7 @@ the SHA-1 ID, and the one line summary. Do not split the tag across multiple
lines, tags are exempt from the "wrap at 75 columns" rule in order to simplify
parsing scripts. For example::
- Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed")
+ Fixes: 6a451e2c5c03 ("ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects", 2024-12-30)
The following ``git config`` settings can be used to add a pretty format for
outputting the above style in the ``git log`` or ``git show`` commands::
@@ -156,12 +156,12 @@ outputting the above style in the ``git log`` or ``git show`` commands::
[core]
abbrev = 12
[pretty]
- fixes = Fixes: %h (\"%s\")
+ fixes = Fixes: %h (\"%s\", %as)
An example call::
- $ git log -1 --pretty=fixes 54a4f0239f2e
- Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed")
+ $ git log -1 --pretty=fixes 6a451e2c5c03
+ Fixes: 6a451e2c5c03 ("ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects", 2024-12-30)
.. _split_changes:
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9eed3683ad76..580d5620ec7d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1228,15 +1228,15 @@ sub git_is_single_file {
}
sub git_commit_info {
- my ($commit, $id, $desc) = @_;
+ my ($commit, $id, $desc, $date) = @_;
- return ($id, $desc) if ((which("git") eq "") || !(-e "$gitroot"));
+ return ($id, $desc, $date) if ((which("git") eq "") || !(-e "$gitroot"));
- my $output = `${git_command} log --no-color --format='%H %s' -1 $commit 2>&1`;
+ my $output = `${git_command} log --no-color --format='%H %s %as' -1 $commit 2>&1`;
$output =~ s/^\s*//gm;
my @lines = split("\n", $output);
- return ($id, $desc) if ($#lines < 0);
+ return ($id, $desc, $date) if ($#lines < 0);
if ($lines[0] =~ /^error: short SHA1 $commit is ambiguous/) {
# Maybe one day convert this block of bash into something that returns
@@ -1253,10 +1253,11 @@ sub git_commit_info {
$id = undef;
} else {
$id = substr($lines[0], 0, 12);
- $desc = substr($lines[0], 41);
+ $desc = substr($lines[0], 41, -11);
+ $date = substr($lines[0], -10);
}
- return ($id, $desc);
+ return ($id, $desc, $date);
}
$chk_signoff = 0 if ($file);
@@ -3214,16 +3215,19 @@ sub process {
my $orig_commit = $2;
my $title;
my $title_has_quotes = 0;
+ my $date;
$fixes_tag = 1;
if (defined $3) {
+ $date = substr($3, -11, 10);
# Always strip leading/trailing parens then double quotes if existing
- $title = substr($3, 1, -1);
+ $title = substr($3, 1, -13);
if ($title =~ /^".*"$/) {
$title = substr($title, 1, -1);
$title_has_quotes = 1;
}
} else {
- $title = "commit title"
+ $title = "commit title";
+ $date = "YYYY-MM-DD";
}
@@ -3234,15 +3238,15 @@ sub process {
my $id_case = not ($orig_commit !~ /[A-F]/);
my $id = "0123456789ab";
- my ($cid, $ctitle) = git_commit_info($orig_commit, $id,
- $title);
+ my ($cid, $ctitle, $cdate) = git_commit_info($orig_commit, $id,
+ $title, $date);
- if ($ctitle ne $title || $tag_case || $tag_space ||
+ if ($ctitle ne $title || $cdate ne $date || $tag_case || $tag_space ||
$id_length || $id_case || !$title_has_quotes) {
if (WARN("BAD_FIXES_TAG",
- "Please use correct Fixes: style 'Fixes: <12 chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) &&
+ "Please use correct Fixes: style 'Fixes: <12 chars of sha1> (\"<title line>\", YYYY-MM-DD)' - ie: 'Fixes: $cid (\"$ctitle\", $cdate)'\n" . $herecurr) &&
$fix) {
- $fixed[$fixlinenr] = "Fixes: $cid (\"$ctitle\")";
+ $fixed[$fixlinenr] = "Fixes: $cid (\"$ctitle\", $cdate)";
}
}
}
--
2.43.0
On Fri, 10 Jan 2025 12:20:09 +0000 Yeking@Red54.com wrote: > The old Fixes tag style is at least 10 years old. It lacks date > information, which can lead to misjudgment. So I added short author date > to avoid this. This make it clear at a glance and reduce > misunderstandings. How can it lead to misjudgment? If you have two or more hashes matching, do you really think they'll have the same subjects? I do not plan on doing this. It's pointless. -- Steve
On 1/10/2025 5:03 AM, Steven Rostedt wrote: > On Fri, 10 Jan 2025 12:20:09 +0000 > Yeking@Red54.com wrote: > >> The old Fixes tag style is at least 10 years old. It lacks date >> information, which can lead to misjudgment. So I added short author date >> to avoid this. This make it clear at a glance and reduce >> misunderstandings. > > How can it lead to misjudgment? If you have two or more hashes matching, do > you really think they'll have the same subjects? > > I do not plan on doing this. It's pointless. > > -- Steve While the addition of the date is a widely used variant within the git community, this was rejected by the kernel community in the past as well. I remember posting fixes tags with the date several years ago and getting push back. I tried to find reference to these discussions but I can't seem to locate them anymore on the archives. I personally find the date helpful as it can help place a commit without needing to take the extra time to do a lookup. However, all of the existing tooling we have for the kernel does not support the date, and I think its not worth trying to change it at this point. It doesn't make sense to break all this tooling for information which is accessible in other forms. Indeed, as long as the hash is sufficiently long, the change of a collision is minimal.
On Fri, 10 Jan 2025 16:21:35 -0800 Jacob Keller <jacob.e.keller@intel.com> wrote: > I personally find the date helpful as it can help place a commit without > needing to take the extra time to do a lookup. I've never found dates to be meaningful. I'm always more concerned about when a commit was added to mainline. Thus the version where the commit was added is very important for me. This is why I keep a bare clone of Linus's tree and commonly do: $ git describe --contains fd3040b9394c v5.19-rc1~159^2~154^2 $ git describe --contains a76053707dbf v5.15-rc1~157^2~376^2~4 I can easily see that a76053707dbf was added in 5.15 and fd3040b9394c was added in 5.19. The amount of work needed to add dates to Fixes tags would greatly exceed the amount of added work someone would need to do to do the above operations if they wanted to know the order of commits. -- Steve
Hi Steven,
On Sat, Jan 11, 2025 at 6:08 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 10 Jan 2025 16:21:35 -0800
> Jacob Keller <jacob.e.keller@intel.com> wrote:
>
> > I personally find the date helpful as it can help place a commit without
> > needing to take the extra time to do a lookup.
>
> I've never found dates to be meaningful. I'm always more concerned about
> when a commit was added to mainline. Thus the version where the commit was
> added is very important for me. This is why I keep a bare clone of Linus's
> tree and commonly do:
>
> $ git describe --contains fd3040b9394c
> v5.19-rc1~159^2~154^2
> $ git describe --contains a76053707dbf
> v5.15-rc1~157^2~376^2~4
>
> I can easily see that a76053707dbf was added in 5.15 and fd3040b9394c was
> added in 5.19. The amount of work needed to add dates to Fixes tags would
> greatly exceed the amount of added work someone would need to do to do the
> above operations if they wanted to know the order of commits.
FTR, while I do not support adding dates to Fixes-tags, I would just
need to make a small modification to my fixes alias:
$ git help fixes
'fixes' is aliased to 'show --format='Fixes: %h ("%s")' -s'
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Sun, 12 Jan 2025 11:54:21 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> FTR, while I do not support adding dates to Fixes-tags, I would just
> need to make a small modification to my fixes alias:
>
> $ git help fixes
> 'fixes' is aliased to 'show --format='Fixes: %h ("%s")' -s'
Hmm, I've just been manually adding the Fixes tags ;-) That's because when
I add a fixes tag, I also do a more in depth analysis to make sure the
commit being tagged is really the cause of the problem. A lot of my fixes
tags are due to very subtle bugs, and a lot of times its a combination of
event that happened.
That said, maybe one day I'll use a script or alias in the future.
-- Steve
On Mon, Jan 13, 2025 at 09:51:01AM -0500, Steven Rostedt wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > $ git help fixes
> > 'fixes' is aliased to 'show --format='Fixes: %h ("%s")' -s'
> Hmm, I've just been manually adding the Fixes tags ;-) That's because when
> I add a fixes tag, I also do a more in depth analysis to make sure the
> commit being tagged is really the cause of the problem. A lot of my fixes
> tags are due to very subtle bugs, and a lot of times its a combination of
> event that happened.
> That said, maybe one day I'll use a script or alias in the future.
I use an alias like that for manually adding fixes tags, to get
everything in the right format.
On Fri, Jan 10, 2025 at 04:21:35PM -0800, Jacob Keller wrote: > However, all of the existing tooling we have for the kernel does not > support the date, and I think its not worth trying to change it at this > point. It doesn't make sense to break all this tooling for information > which is accessible in other forms. Indeed, as long as the hash is > sufficiently long, the change of a collision is minimal. And if it isn't long enough, tools like: https://lore.kernel.org/r/20241226220555.3540872-1-sashal@kernel.org can help figure it out as well. thanks, greg k-h
On Fri, Jan 10, 2025 at 12:20:09PM +0000, Yeking@Red54.com wrote:
> From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
>
> The old Fixes tag style is at least 10 years old. It lacks date
> information, which can lead to misjudgment. So I added short author date
> to avoid this. This make it clear at a glance and reduce
> misunderstandings.
>
> For example:
>
> Old Fixes tag style:
> * Fixes: fd3040b9394c ("net: ethernet: Add driver for Sunplus SP7021")
> * Fixes: a76053707dbf ("dev_ioctl: split out ndo_eth_ioctl")
> This will make people mistakenly think that "a76053707dbf" broke
> "fd3040b9394c".[1]
>
> New Fixes tag style:
> * Fixes: fd3040b9394c ("net: ethernet: Add driver for Sunplus SP7021", 2022-05-08)
> * Fixes: a76053707dbf ("dev_ioctl: split out ndo_eth_ioctl", 2021-07-27)
> This makes it clear that the newly introduced driver did not follow the
> existing changes.
Please no, you will break all of our tooling and scripts that parse
these types of fields. The git commit id and commit header is all we
need to properly determine this type of information, no need to add a
date in here at all.
There's no issue with "making things clear" that I can tell, and no,
listing multiple fixes lines does NOT mean that a previous line broke
something at all. It just means that a single commit happened to fix
multiple commits (which frankly is a rare thing, and one that our
scripts already have a hard enough time dealing with...)
So I don't think you need to add a date here. Dates also really do not
mean much with commits, what matters is what release a commit is in, not
when it was originally made. We have commits that take years to show up
in a release, so if you only look at a date, you will be mistaken many
times as it's not showing what came before what many times (i.e. we
apply commits out-of-date-order all the time.)
thanks,
greg k-h
On Thu, Jan 09, 2025 at 02:05:52AM +0000, Yeking@Red54.com wrote:
> From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
>
> ndo_do_ioctl is no longer called by the device ioctl handler,
> so use ndo_eth_ioctl instead. (found by code inspection)
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
There are a couple of things which would of been nice to add, but the
patch can be accepted anyway.
One is a bit more details in the commit message about when
ndo_do_ioctl is actually used, just to help make it clearer why the
code is wrong and the patch is correct. You could of quoted
a76053707dbf:
Most users of ndo_do_ioctl are ethernet drivers that implement
the MII commands SIOCGMIIPHY/SIOCGMIIREG/SIOCSMIIREG, or hardware
timestamping with SIOCSHWTSTAMP/SIOCGHWTSTAMP.
Separate these from the few drivers that use ndo_do_ioctl to
implement SIOCBOND, SIOCBR and SIOCWANDEV commands.
Also, if i find a bug like this, i often wounder if there are more
instances of the same bug somewhere else. I did a quick grep and it
does seem to be the only case. If you did the same check, it would be
nice to mention this in the commit message, maybe below the --- marker
so it does not become part of the permanent history in git.
Andrew
© 2016 - 2025 Red Hat, Inc.