Hi Frank, here the link. https://patchwork.kernel.org/project/netdevbpf/patch/AS1PR03MB8189F6C1F7DF7907E38198D382672@AS1PR03MB8189.eurprd03.prod.outlook.com/
Some information additional (or shorter) to the readme you already got in the Mailinglist:
Look for previous commits on this file to get the right subsystem prefix and gives you some examples how commits should look like…
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/log/drivers/net/phy/sfp.c
As you see,commits here are like this
net: sfp: add support for xyz module
you can change commit with
git commit --amend -s
If it is your topmost commit (i guess it is in your tree). The s flag adds the signed-off-by if not already there.
Then add a commit message explaining why this patch is needed (see also previous commits as example).
Then use “git format-patch HEAD~1” and check the resulting file with scripts/checkpatch.pl and use get_maintainers.pl to get recipients (maintainer,reviewers and mailinglists only - exclude commit signers). Then you can use git send-email to send the patch to the adresses you’ve got
Hi Frank, I struggled all day in order to understand how to proceed without success. Unfortunately I started from patch already prepared without creating it from scratch and mailed it without applying it in the system. However I cloned kernel/git/netdev/net-next.git - Netdev Group's -next networking tree and tryed to apply the patch without success (errors reported which I could not understand). It is still not clear to me how to create (diff ?) the patch and/or apply it, when to commit and how to sned it to the maintainers mailing list. I read documents suggested, but still need some very basic help in order to understand how to proceed. Still not clear to me if I can do the requested changes directly on the patch file and send it to the maintaines mailing list or if I have to create, apply, commit it in the system and use the git commands to proceed. Your and Daniel’s help would be much apreciated.
your patch does not apply because you have used spaces instead of tabs for the existing lines (Lantech and UBNT SFP)
then i would align the quirks by vendor so it should imho be near the other OEM quirks
so codechange is more like this:
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index f75c9eb3958e..1a447e3567c8 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -506,6 +506,9 @@ static const struct sfp_quirk sfp_quirks[] = {
SFP_QUIRK_M("OEM", "SFP-2.5G-T", sfp_quirk_oem_2_5g),
SFP_QUIRK_F("OEM", "RTSFP-10", sfp_fixup_rollball_cc),
SFP_QUIRK_F("OEM", "RTSFP-10G", sfp_fixup_rollball_cc),
+ // DFP-34X-2C2 GPON ONU supports 2500base-X
+ SFP_QUIRK_M("OEM", "DFP-34X-2C2", sfp_quirk_2500basex),
+
SFP_QUIRK_F("Turris", "RTSFP-10", sfp_fixup_rollball),
SFP_QUIRK_F("Turris", "RTSFP-10G", sfp_fixup_rollball),
};
so now look how the commits look like for older sfp support
$ git log --oneline -10 drivers/net/phy/sfp.c
e9301af385e7 net: sfp: fix PHY discovery for FS SFP-10G-T module
bb1afee98466 net: sfp: Convert to platform remove callback returning void
2f3ce7a56c6e net: sfp: rework the RollBall PHY waiting code
dd9d75fcf0f4 net: phy: fill in missing MODULE_DESCRIPTION()s
5ffe330e40bd net: sfp: improve Nokia GPON sfp fixup
e184e8609f8c net: sfp: re-implement ignoring the hardware TX_FAULT signal
e27aca3760c0 net: sfp: add quirk for FS's 2.5G copper SFP
d387e34fec40 net: sfp: add quirk for Fiberstone GPON-ONU-34-20BI
f4bf467883f2 net: phy: move marking PHY on SFP module into SFP code
ac2e8e3cfe48 net: sfp: add support for HXSX-ATRI-1 copper SFP+ module
e.g. e27aca3760c0 looks good so far, so use e.g. git show e27aca3760c0 to look how commit message is written (you can copy the text and modify it to match your SFP without the indent added by git show).
git commit -s drivers/net/phy/sfp.c
something like this (signed-off-by is added with the -s switch of git commit):
net: sfp: add quirk for DFP-34X-2C2 GPON ONU SFP Add a quirk for a GPON SFP that identifies itself as "OEM", "DFP-34X-2C2". This module's PHY is inaccessible, and can only run at 2500base-X.
then you have the commit like it should be on top of your tree (you can use “git show” without ref to see it)…lets make a patch from it, check this and get maintainers
$ git format-patch HEAD^
0001-net-sfp-add-quirk-for-DFP-34X-2C2-GPON-ONU-SFP.patch
$ scripts/checkpatch.pl 0001-net-sfp-add-quirk-for-DFP-34X-2C2-GPON-ONU-SFP.patch
total: 0 errors, 0 warnings, 0 checks, 9 lines checked
0001-net-sfp-add-quirk-for-DFP-34X-2C2-GPON-ONU-SFP.patch has no obvious style problems and is ready for submission.
$ scripts/get_maintainer.pl 0001-net-sfp-add-quirk-for-DFP-34X-2C2-GPON-ONU-SFP.patch
...
you can add these adresses directly to patch-file or inserting them when running git send-email (maybe needs to be installed - sudo apt install git-email in debian/ubuntu)
to add to Patch-File you can open your created patch in editor of your choice and add To: and Cc: headers between Date and Subject without the part in parenthesis (no blank line). add a comma at the end of line except the last recipient per header and a space before emails not prefixed by email-Header-Tag
so it should look like (maintainers in “To”, all other - except commit signers - in “Cc”):
Date: Sat, 6 Jan 2024 12:31:35 +0100
To: Russell King <email>,
Andrew Lunn <email>,
Heiner Kallweit <email>,
"David S. Miller" <email>,
Eric Dumazet <email>,
Jakub Kicinski <email>,
Paolo Abeni <email>
Cc: [email protected],
[email protected]
Subject: [PATCH] net: sfp: add quirk for DFP-34X-2C2 GPON ONU SFP
after saving you can send your patch
$ git send-email 0001-net-sfp-add-quirk-for-DFP-34X-2C2-GPON-ONU-SFP.patch
0001-net-sfp-add-quirk-for-DFP-34X-2C2-GPON-ONU-SFP.patch
To whom should the emails be sent (if anyone)?
Message-ID to be used as In-Reply-To for the first email (if any)?
...
From: Frank Wunderlich <email>
To: Russell King <email>,
Andrew Lunn <email>,
Heiner Kallweit <email>,
"David S. Miller" <email>,
Eric Dumazet <email>,
Jakub Kicinski <email>,
Paolo Abeni <email>
Cc: Frank Wunderlich <enail>,
[email protected],
[email protected]
Subject: [PATCH] net: sfp: add quirk for DFP-34X-2C2 GPON ONU SFP
Date: Sat, 6 Jan 2024 12:40:12 +0100
Message-Id: <20240106114012.10761-1-...>
X-Mailer: git-send-email 2.34.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
The Cc list above has been expanded by additional
addresses found in the patch commit message. By default
send-email prompts before sending whenever this occurs.
This behavior is controlled by the sendemail.confirm
configuration setting.
For additional information, run 'git send-email --help'.
To retain the current behavior, but squelch this message,
run 'git config --global sendemail.confirm auto'.
Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll):
make sure the To- and Cc-headers are displayed correctly (not merged 2 adresses or similar)
you may need to setup your mail-provider before if not done already
$ nano ~/.gitconfig
and add a section with you data:
[sendemail]
#from = Name <email> #optional if not matching the senders email
smtpencryption = ssl
smtpserver = <mailserver>
smtpServerPort = 465
smtpuser = <loginuser>
composeencoding = UTF-8
Hi Frank, thanks very much for your complete explanationss. This made me clear on actions to be performed. So:
- I made changes on the file deivers/net/phy/sfp.c
- I committed the changes using the format message needed for netdev
- I formatted the patch using git format-patch as per instructions
- I got the list of the recipients to be emailed
- I added the To and the Cc to the file
- I sent the email using git send-email
- Patch showed up at netdev + bpf
Let me know if any further action is needed.
Found patch here: net: sfp: add quirk for DFP-34X-2C2 GPON ONU SFP - Patchwork
Forgot to mention that new versions need a version increasing…you get it when using format-patch with -v flag (-v2 creates version 2 where patches named v2-000*.patch),sorry for telling this.
Also noted that you have a indent beginning with line 2 of message and a typo in “enabled”. Have you make sure your message has max 75 chars per line (checkpatch should report this)?
I guess you will need v3. Don’t worry,nobody is perfect…i made patches with more than 8 versions 
Else once patch is out,wait for comments and fix the problems told you,if unsure ask how you should fix it (but after thinking about it)
Hi Frank, the 75 chars is ok. I saw the typo ater the patch was sent. I think the name of the patch is different from the one posted today. Is it the warning related to the [PATCH] instead of [PATCH net-next] in the subject? In that case what is needed to do? Edit the patch file changing typo and adding net-next and sending? or restart from recreation the patch with git format-patch ?
You should always change the commit and recreate the atch as described above,but first wait for responses of your v2 patch…i would like to know if the comment line is ok or should be dropped
For length…if your commits headline is below 75chars it is ok…the [Patch …] Is dropped when patch is applied…it will also contain the version when you add it…it makes it easier to see changes and drop the old versions from list
As you got response from russel,you can prepare changes he suggested…but i woult wait at least 24h for others to comment
You can set the tree (net-next here) by providing --subject-prefix (iirc,maybe do “git format-patch --help”)
If you post new version please add a version log after the tags (below signed-off-by atm) separated by — line
Something like this:
Signed-off-by: Sergio Palumbo <...>
---
Changes:
V3:
- fix identation of commit message
- add target tree (net-next)
- describe 1000 vs 2500baseX switch
V2:
- add commit message and SoB
- add subsystem prefix
---
drivers/net/phy/sfp.c | 3
Hi Frank, I got comments from Russel and now is clear changes I have to do. Unfortunately there are now 2 new variables:
- I discovered that I have two different versions of the modle DFP-34X-2C2 one with vedor ID “ODI” and another with vendor ID “OEM” so I already compiled a new version of the OWRT for Banana PI R3 with the patch to accomodate both vendor IDs and is correctly working with the 2 modules.
- the net-next is closed due to the cycle and Russel told that this it should be the correct tree where to submit the patch. As far as the point 1. Do I need to create a completely new Patch with V1? or do I have to create a V2 includiing also the second? As far as pooint 2 better to wait the net-next is opened again or ppost the new patch in the net tree? When sending a patch with a new version will the patchwork create a new record? Thanks Sergio
1 quirk per patch, you can create a series, but imho it is easier to send them both alone (new one as v1, the old as v3 when changed the things suggested) as they do not depend on each other, right?
no as russel told you, new features are net-next (this is a special handling for the networking subsystem as it is very large subsystem with much patches not clearly fixes or feature-updates).
yes, wait for net-next to be opened…it should be only ~1 week (Monday in 1 week, 2 weeks after release of 6.7 final where the current next-trees are merged into torvalds as 6.8-rc1).
but you can test how to get the net-next-flag in (afair git format-patch --subject-prefix), or do you have already?
Frank, Daniel, can you please help me in the discussion with net-dev group and Russell who does not want to accept the patch for the two modules because he is saying that it has to be tested in case it is used with an linux host working only at 1000base-X and the quirk installed. I tested with Banana PI R3 without the patch and module showing up to linux at 1000base-X only and setting speed at 1000 Mb. I tested with Banana PI R3 with the patch and module showing up at both 1000base-X and 2500base-X setting speed at 2500 Mb I think this is same situation of Huawei MA5671A and Fiberstone GPON-ONU-34-20BI. This two modules require “sfp_quirk_2500basex” and “sfp_fixup_ignore_tx_fault” quirks, while DFP-34X-2C2 only requires “sfp_quirk_2500basex” Is there any possibility that after the quirk there will be any situation where there can be problems? I’m really confused, but I’m sure the patch is working and I tested in all situation I could. Thanks for your help. Sergio
If i understood correctly the module now works in both modes, before it was only 1000base-X, right?
russel still want to avoid that this quirk will be changed another time so it should include all modes the SFP supports. i do not have a ONT SFP and not know much about them, so except the commit-Message itself i cannot help much…when posting to mainline please always refer to mainline code, not openwrt.
for commit-message you still need repost because of the unwanted indentation. Then write which modes the SFP supports from datasheet and maybe which modes you have tested. Together with the main problem (sfp is not working because it needs mode xyz not set in eeprom) in front imho it should be OK.
only one problem i read out of russels comments:
it will cause these modules to regress when they are in the manufacturer default state when used with a host that supports both 1000base-X and 2500base-X
so maybe you now force the SFP to 2500Base-X so it does not work with 1000Base-X which is a regression for other users, i’m not sure, if i understand it correctly and if you can add multiple modes.
Hi Frank, I tested on Banana PI R3 host capable of both 1000-X and 2500-X without the quirk the system was working at 1000-X with both settings LAN_SDS_MODE=1 (1000-X) and LAN_SDS_MODE=6 (2500-X). The module was not showing up at 2500-X. After the quirk the module started showing up at 2500-X. The system was working at 2500-X with both settings LAN_SDS_MODE=1 (1000-X) and LAN_SDS_MODE=6 (2500-X). Being BPI R3 working at both 1000-X and 2500-X after the quirk is always working at 2500-X independently by the settings in the module. I do not have a machine with an sfp working at maximum 1000-X to test if after the quirk the module can work at 1000-X I tried to change the speed usign ethtool but when trying to change speed I get an error message, However we had a lot of messages on this with Russell and at the end he asked me:
Hi Sergio,
I did ask for the kernel messages from a specific scenario:
- host that supports 1000base-X and 2500base-X with your quirk
- SFP inserted with LAN_SDS_MODE=1
What I expet to see in the kernel messages is that the system will
use 2500base-X, and a failure.
You claim that the kernel will link at 1000base-X. There is no
mechanism in the kernel for this to happen, and I believe that
if you look at the kernel messages, this will prove my point.
I asked for it with a kernel that has asked for it with a kernel that has #define DEBUG in phylink.c, but I see no debug messages from phylink in your quoted output.
Unfortunately I do not know how to do the test with a kernel that has #define DEBUG in phylink.c. I asked help on how to do it in openwrt and started saying openwrt different from the main etc… Do you think I can do the test with this debug? Any idea on how to test an host having 1000-X only? I’m quite sure that the other 2 SFP GPON ONT already quirked are having same behavior of mine, but cannot be sure. Any comment is welcome.
Have you tried adding the define to phylink.c in your openwrt code? Afair it needs to be before the includes as one of them enables the dev_dbg.
For 1000baseX only host,maybe you can change mtk mac driver to drop 2500baseX mode on mt7986 temporarily 
OK now copilig with #define DEBUG on top in
/build_dir/toolchain-aarch64_cortex-a53__gcc-112.30_musl/linux-5.15.137/drivers/net/phy/phylink.c
Let’s see what will happen.
So the real question, if the module should work at 2500basex and 1000basex (somehow it sorts out which to use), then why doesn’t it connect at 2500basex without quirk? What exactly happens / is set or cleared in the quirk?
Before there was someone with a module here on the forum, that switched between the 2 interface modes, until it found out which to use. How does your module work?
Also keep in mind, that the R3 does not support inband auto-negotiation at 2500basex, but it does at 1000basex. However, phylink still does not know that the pcs/mac doesn’t support it at 2500basex, so has it enabled. This causes problems.
Since these modules connect as optical modules, the autoneg set with ethtool only applies to the autoneg between module and mac. So inband an can be disabled with ethtool.
I have been working on a phylink patch to handle this, but it also isn’t going to be accepted upstream. This case is much more complex and my simple hacky patch will not be good enough for all hardware.
If at all this is the problem here…
Hello Eric, I think the problem is that the EEPROM does not provide to linux the correct info and the module without the quirk is showing up at 1000basex only:
root@OpenWrt:~# ethtool eth1
Settings for eth1:100
Supported ports: [ FIBRE ]
Supported link modes: 1000baseX/Full
Supported pause frame use: Symmetric Receive-only
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: 1000baseX/Full
Advertised pause frame use: Symmetric Receive-only
Advertised auto-negotiation: Yes
Advertised FEC modes: Not reported
Link partner advertised link modes: 1000baseX/Full
Link partner advertised pause frame use: Symmetric Receive-only link modes:
Link partner advertised auto-negotiation: Yes
Link partner advertised FEC modes: Not reported
Speed: 1000Mb/s
Duplex: Full
Auto-negotiation: on
Port: FIBRE
PHYAD: 0
Transceiver: internal
Current message level: 0x000000ff (255)
drv probe link timer ifdown ifup rx_err tx_err
Link detected: yes
with the quirk the module is showing up at both 2500basex and 1000basex
root@OpenWrt:~# ethtool eth1
Settings for eth1:
Supported ports: [ FIBRE ]
Supported link modes: 2500baseX/Full
1000baseX/Full
Supported pause frame use: Symmetric Receive-only
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: 2500baseX/Full
Advertised pause frame use: Symmetric Receive-only
Advertised auto-negotiation: Yes
Advertised FEC modes: Not reported
Speed: 2500Mb/s
Duplex: Full
Auto-negotiation: on
Port: FIBRE
PHYAD: 0
Transceiver: internal
Current message level: 0x000000ff (255)
drv probe link timer ifdown ifup rx_err tx_err
Link detected: yes
After the quirk the module always connecting at 2500basex even if I try to set the module at 1000X
Tried to decrease speed to 1000 by using ethtool:
root@OpenWrt:~# ethtool -s eth1 speed 1000
netlink error: link settings update failed
netlink error: Invalid argument
it seems not to be possible.
I am not very familiar with ont’s, but you also have settings in the ont. What it reports as ‘eeprom’ is not so static on these things.
What is exactly in the quirk that you point it to?
Which code and what does it set/clear?
The quirk is not a areal patch to the codeand I do not know how it works at low level. There is a place in drivers/net/phy/sfp.c file where you can declare vendor and part number of the module and to force 2500base-X. There is a list of the modules and the possible rules to be applied and the patch is consisting in adding to the list the above mentoned paramters for a new module. I did it and the module is working, but it seems regression is needed in order to besure that who is currently using the module without the quirk is not suffering problems when the quirk is applied. Hope this clarifies.
Which source do you use? There are many different versions. And what is the code in sfp_quirk_2500basex() in this source?