+ Alexandre Torgue <alexandre.torgue@xxxxxx> (Owner of phy-miphy28lp.c)
+ Loc Ho <lho@xxxxxxx> (Owner of phy-miphy28lp.c)
On 10/22/2015 05:56 PM, Murali Karicheri wrote:
On 10/22/2015 01:48 PM, Russell King - ARM Linux wrote:
On Thu, Oct 22, 2015 at 11:05:26AM -0400, Murali Karicheri wrote:
On 10/21/2015 08:56 AM, WingMan Kwok wrote:
On TI's Keystone platforms, several peripherals such as the
gbe ethernet switch, 10gbe ethernet switch and PCIe controller
require the use of a SerDes for converting SoC parallel data into
serialized data that can be output over a high-speed electrical
interface, and also converting high-speed serial input data
into parallel data that can be processed by the SoC. The
SerDeses used by those peripherals, though they may be different,
are largely similar in functionality and setup.
------------Cut-------------------------------------------------
Documentation/devicetree/bindings/phy/ti-phy.txt | 239 +++
drivers/pci/host/pci-keystone.c | 24 +-
drivers/pci/host/pci-keystone.h | 1 +
drivers/phy/Kconfig | 8 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-keystone-serdes.c | 2366
++++++++++++++++++++++
6 files changed, 2629 insertions(+), 10 deletions(-)
create mode 100644 drivers/phy/phy-keystone-serdes.c
Kishon, Bjorn
Who will pick this up? Do we have time to get this in 4.4?
I've been avoiding this since my initial comments, but if you're wanting
to get it into v4.4, then I have to say something.
Russell,
I saw you have raised this point earlier against v1 of the patch series.
I have responded as below (cut-n-pasted from that email)
"The serdes on K2 are re-used on multiple hardware blocks as already
indicated in this thread. It has got multiple lanes, each lane can be
enabled/disabled, shutdown etc. Isn't generic phy framework added to
support this type of hardware block? I see some enhancements needed for
K2 serdes to support monitoring the serdes link and providing a status
to the higher layer device. So I am not clear what different way you
would like to handle serdes drivers? Why do you need a new framework?
"
KISHON VIJAY had responded saying
"The PHY framework (in drivers/phy/) already provides a standard
interface to be used by the controller drivers no?"
But I have not seen your response to these questions from us. v2 and v3
has gone by and since all of the outstanding comments have been
addressed and you have not responded to our questions, I thought this
can be merged for 4.4. Good to see you have responded now :)
Again, there's other SoCs out there which have serdes. Adding 2.5k of
lines for vendor serdes implementations does not scale - this needs to
be re-thought in a way which reduces the code maintanence burden.
Other SoCs like Marvell Armada have serdes links which can be configured
between SATA, PCIe and Gbe. Should Armada end up adding another 2.5k
lines to support their device too? What happens when we have 10 of
these, and we have 25k lines of code here?
Again, this does not scale. Please look at what can be done to reduce
the code size when other implementations come along.
Well, per our understanding, this driver is a Generic phy driver and we
have implemented a device driver based on Generic Phy API. This driver
is expected to support all of the 3 peripherals :- PCIe, 1G and 10G
Ethernet. You have mentioned about Marvell & Armada . Did Marvell post
any patch already? Without seeing their code, how will we be able to
investigate what can be factored out to a generic serdes core driver? By
making this statement, I assume you are still considering using the
Generic Phy driver framework for SerDes drivers. Don't you?
I did a search in the phy folder and these are the top ones that came
out in terms of number of lines of code after Phy-core.c.
ls *.[ch] | xargs wc -l | sort -n
943 phy-core.c
1279 phy-miphy28lp.c
1735 phy-xgene.c
2367 phy-keystone-serdes.c
So focusing on the top 3 drivers (including keystone serdes) under phy.
phy-xgene.c
-----------
Looking at other drivers under drivers/phy, I could find phy-xgene.c
which is close Keystone SerDes driver (. This is called APM X-Gene
Multi-Purpose PHY driver. It defines following mode per the driver code
MODE_SATA = 0, /* List them for simple reference */
MODE_SGMII = 1,
MODE_PCIE = 2,
MODE_USB = 3,
MODE_XFI = 4,
But seems to support only MODE_SATA. From the code, it appears, this
driver is expected to be enhanced in the future to support additional
modes. I have copied the author to this email to participate in this
discussion.
Keystone SerDes supports following modes
----------------------------------------
KSERDES_PHY_SGMII,
KSERDES_PHY_XGE,
KSERDES_PHY_PCIE,
KSERDES_PHY_HYPERLINK,
KSERDES_PHY_SRIO
And phy-miphy28lp.c
---------------------
+#define PHY_TYPE_SATA 1
+#define PHY_TYPE_PCIE 2
+#define PHY_TYPE_USB2 3
+#define PHY_TYPE_USB3 4
Keystone SerDes hardware is highly parameterized. The init has following
steps:-
- Configure the Phy to one of the mode (SATA,SGMII,PCIE,USB,XFI)
- Configure the Phy to the specific mode
- Configure N lanes for the selected mode
- Enable N Lanes
So at a high level, I can imagine these kind of Phys require additionally
- Enable/Disable Lane
- check lane status periodically
So there is a scope for enhancing the Phy core API to handle these kinds
of phy ops. This might help to re-use some code. But at the lower level
driver, we still need to write to vendor specific registers and
configure the SerDes which is the major part of the driver and that
still will be a major part of these drivers.
I would also like to hear from Kishon (Maintainer) on his ideas for
Generic Phy driver to support these kind of SerDes hardwares.
I think it is fair to ask to merge the Keystone SerDes driver right now
as we have spend considerable time reviewing the current series and
taken care of all other outstanding comments. We are most happy to
enhance the Phy core framework to help re-use code across the above and
future SerDes driver that supports multiple modes.
Or do you have some other ideas that you would like to share?
Murali
(I am aware that guys working on Marvell Armada are looking into this
problem - but I know they're ready to post anything yet.)
--
Murali Karicheri
Linux Kernel, Keystone
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
