Re: [PATCH v1 00/23] phy: marvell: Sync Armada 3k/7k/8k SERDES code with Marvell version
On Wed, 5 May 2021 11:19:13 +0200 Pali Rohár wrote: > > "bubt" is special and cannot be changed easily without breaking update > > scripts using it AFAICT. As it's pretty old and used in the Marvell code > > base for quite some time - including all the documentation about > > updating. > > I see. This needs to say in current form. bubt can theoretically be implemented for other platforms. Its purpose is to update u-boot. This is useful for other platforms. Marek
Re: [PATCH v1 00/23] phy: marvell: Sync Armada 3k/7k/8k SERDES code with Marvell version
On Wednesday 05 May 2021 09:00:57 Stefan Roese wrote: > Hi Pali, > > On 05.05.21 00:28, Pali Rohár wrote: > > On Thursday 29 April 2021 11:00:17 Stefan Roese wrote: > > > Hi Marek, > > > > > > (Added Tom and Simon to Cc) > > > > > > On 29.04.21 10:27, Marek Behun wrote: > > > > On Thu, 29 Apr 2021 08:46:36 +0200 > > > > Stefan Roese wrote: > > > > > > > > > > phy: marvell: add RX training command > > > > > > > > > > Applied to u-boot-marvell/master > > > > > > > > > > Thanks, > > > > > Stefan > > > > > > > > Stefan, do you think it would make sense to at least create a special > > > > mechanism for these platform commands? For example via a top-level > > > > command "plat", i.e. instead of > > > > > rx_training params > > > > we would call > > > > > plat rx_training params > > > > > > > > The plat command could list all registered platform specific commands... > > > > > > Not sure. If you want to split it up, then we would perhaps also > > > need to add a mechanism for board specific commands as well. E.g. for > > > commands not common for a platform but only for specific boards. My > > > feeling is that this makes things overly complex. And I also don't see > > > a real problem with the current "flat" structure of these commands > > > being "global". Again, I mention the many already existing board and > > > platform specific commands in current mainline. > > > > > > Perhaps other people can comment on this use / introduction of > > > platform specific U-Boot commands? > > > > > > BTW: Again, we can definitely rename this specific "rx_training" > > > command, if you feel this is absolutely misleading. IIRC, then I already > > > made a suggestion for this. > > > > Hello! "rx_training" is definitely ambiguous and I strongly suggest to > > rename this command (if is going to be merged in current form). > > It's already in mainline. I'll probably send a patch to rename it, > please see below... > > > My first impression was that this command is suppose to do some DDR3/4 > > training sequence but it is doing something totally different. > > > > I'm also not a big fan of these custom vendor specific/proprietary > > commands. And I rather would like to see generic command with an API so > > other boards and vendors could implement it too. > > > > But if this comphy rx training code is something specific to Marvell > > platforms and there is no other platform which needs such abstraction > > then lets have it as custom vendor specific command. > > > > I hope that Tom or Simon have better knowledge of U-Boot code and > > hardware on which is U-Boot running and can say if there are other > > platforms for such command or not. > > > > And if "plat" command is too complex for this, what about renaming this > > command to something like "mvebu_comphy_rx_training" or something > > similar? To express that it is Marvell specific and also mention that it > > is for comphy. And not for ddr, uart or ethernet phy. > > No other platform than "MVEBU" can select this command. So adding > "mvebu" seems superflous to me. But it would make it clear that it's > platform specific never the less. I agree that "comphy" makes > perfect sense to avoid confusion (mixup with DDR3/4) here. I mean if there is another platform for which such command could be implemented in future (not what is currently implemented/supported) or if comphy rx training "feature" is a8k specific. > > Same applies for Marvell command "bubt" which is already present in > > U-Boot codebase. > > "bubt" is special and cannot be changed easily without breaking update > scripts using it AFAICT. As it's pretty old and used in the Marvell code > base for quite some time - including all the documentation about > updating. I see. This needs to say in current form. > > It has totally insane name as abbreviation of Burn > > UBooT and moreover on A3720 is does not even work with U-Boot image but > > rather with "big" image in specific custom format which contains > > concatenation of Cortex-M3 Secure Firmware, Cortex-A53 Trusted Firmware > > and U-Boot. And I think this "bubt" is example of command which should > > not be vendor specific but rather generic U-Boot command as its purpose > > is to update vendor specific boot image stored in nand/eMMC either via > > TFTP or from uSD card. So this command could have been called "fwupdate" > > or similar to express that is updated vendor specific firmware or U-Boot > > bootloader in vendor specific format (if U-Boot needs to have some > > encapsulation) for current board to current "boot device/partition". > > Yes, it would be nice to have a common / generic command for such an > update process with multiple options (storage device etc). > > Thanks, > Stefan
Re: [PATCH v1 00/23] phy: marvell: Sync Armada 3k/7k/8k SERDES code with Marvell version
Hi Pali, On 05.05.21 00:28, Pali Rohár wrote: On Thursday 29 April 2021 11:00:17 Stefan Roese wrote: Hi Marek, (Added Tom and Simon to Cc) On 29.04.21 10:27, Marek Behun wrote: On Thu, 29 Apr 2021 08:46:36 +0200 Stefan Roese wrote: phy: marvell: add RX training command Applied to u-boot-marvell/master Thanks, Stefan Stefan, do you think it would make sense to at least create a special mechanism for these platform commands? For example via a top-level command "plat", i.e. instead of > rx_training params we would call > plat rx_training params The plat command could list all registered platform specific commands... Not sure. If you want to split it up, then we would perhaps also need to add a mechanism for board specific commands as well. E.g. for commands not common for a platform but only for specific boards. My feeling is that this makes things overly complex. And I also don't see a real problem with the current "flat" structure of these commands being "global". Again, I mention the many already existing board and platform specific commands in current mainline. Perhaps other people can comment on this use / introduction of platform specific U-Boot commands? BTW: Again, we can definitely rename this specific "rx_training" command, if you feel this is absolutely misleading. IIRC, then I already made a suggestion for this. Hello! "rx_training" is definitely ambiguous and I strongly suggest to rename this command (if is going to be merged in current form). It's already in mainline. I'll probably send a patch to rename it, please see below... My first impression was that this command is suppose to do some DDR3/4 training sequence but it is doing something totally different. I'm also not a big fan of these custom vendor specific/proprietary commands. And I rather would like to see generic command with an API so other boards and vendors could implement it too. But if this comphy rx training code is something specific to Marvell platforms and there is no other platform which needs such abstraction then lets have it as custom vendor specific command. I hope that Tom or Simon have better knowledge of U-Boot code and hardware on which is U-Boot running and can say if there are other platforms for such command or not. And if "plat" command is too complex for this, what about renaming this command to something like "mvebu_comphy_rx_training" or something similar? To express that it is Marvell specific and also mention that it is for comphy. And not for ddr, uart or ethernet phy. No other platform than "MVEBU" can select this command. So adding "mvebu" seems superflous to me. But it would make it clear that it's platform specific never the less. I agree that "comphy" makes perfect sense to avoid confusion (mixup with DDR3/4) here. Same applies for Marvell command "bubt" which is already present in U-Boot codebase. "bubt" is special and cannot be changed easily without breaking update scripts using it AFAICT. As it's pretty old and used in the Marvell code base for quite some time - including all the documentation about updating. It has totally insane name as abbreviation of Burn UBooT and moreover on A3720 is does not even work with U-Boot image but rather with "big" image in specific custom format which contains concatenation of Cortex-M3 Secure Firmware, Cortex-A53 Trusted Firmware and U-Boot. And I think this "bubt" is example of command which should not be vendor specific but rather generic U-Boot command as its purpose is to update vendor specific boot image stored in nand/eMMC either via TFTP or from uSD card. So this command could have been called "fwupdate" or similar to express that is updated vendor specific firmware or U-Boot bootloader in vendor specific format (if U-Boot needs to have some encapsulation) for current board to current "boot device/partition". Yes, it would be nice to have a common / generic command for such an update process with multiple options (storage device etc). Thanks, Stefan
Re: [PATCH v1 00/23] phy: marvell: Sync Armada 3k/7k/8k SERDES code with Marvell version
On Thursday 29 April 2021 11:00:17 Stefan Roese wrote: > Hi Marek, > > (Added Tom and Simon to Cc) > > On 29.04.21 10:27, Marek Behun wrote: > > On Thu, 29 Apr 2021 08:46:36 +0200 > > Stefan Roese wrote: > > > > > > phy: marvell: add RX training command > > > > > > Applied to u-boot-marvell/master > > > > > > Thanks, > > > Stefan > > > > Stefan, do you think it would make sense to at least create a special > > mechanism for these platform commands? For example via a top-level > > command "plat", i.e. instead of > >> rx_training params > > we would call > >> plat rx_training params > > > > The plat command could list all registered platform specific commands... > > Not sure. If you want to split it up, then we would perhaps also > need to add a mechanism for board specific commands as well. E.g. for > commands not common for a platform but only for specific boards. My > feeling is that this makes things overly complex. And I also don't see > a real problem with the current "flat" structure of these commands > being "global". Again, I mention the many already existing board and > platform specific commands in current mainline. > > Perhaps other people can comment on this use / introduction of > platform specific U-Boot commands? > > BTW: Again, we can definitely rename this specific "rx_training" > command, if you feel this is absolutely misleading. IIRC, then I already > made a suggestion for this. Hello! "rx_training" is definitely ambiguous and I strongly suggest to rename this command (if is going to be merged in current form). My first impression was that this command is suppose to do some DDR3/4 training sequence but it is doing something totally different. I'm also not a big fan of these custom vendor specific/proprietary commands. And I rather would like to see generic command with an API so other boards and vendors could implement it too. But if this comphy rx training code is something specific to Marvell platforms and there is no other platform which needs such abstraction then lets have it as custom vendor specific command. I hope that Tom or Simon have better knowledge of U-Boot code and hardware on which is U-Boot running and can say if there are other platforms for such command or not. And if "plat" command is too complex for this, what about renaming this command to something like "mvebu_comphy_rx_training" or something similar? To express that it is Marvell specific and also mention that it is for comphy. And not for ddr, uart or ethernet phy. Same applies for Marvell command "bubt" which is already present in U-Boot codebase. It has totally insane name as abbreviation of Burn UBooT and moreover on A3720 is does not even work with U-Boot image but rather with "big" image in specific custom format which contains concatenation of Cortex-M3 Secure Firmware, Cortex-A53 Trusted Firmware and U-Boot. And I think this "bubt" is example of command which should not be vendor specific but rather generic U-Boot command as its purpose is to update vendor specific boot image stored in nand/eMMC either via TFTP or from uSD card. So this command could have been called "fwupdate" or similar to express that is updated vendor specific firmware or U-Boot bootloader in vendor specific format (if U-Boot needs to have some encapsulation) for current board to current "boot device/partition". > Thanks, > Stefan
Re: [PATCH v1 00/23] phy: marvell: Sync Armada 3k/7k/8k SERDES code with Marvell version
Hi Marek, (Added Tom and Simon to Cc) On 29.04.21 10:27, Marek Behun wrote: On Thu, 29 Apr 2021 08:46:36 +0200 Stefan Roese wrote: phy: marvell: add RX training command Applied to u-boot-marvell/master Thanks, Stefan Stefan, do you think it would make sense to at least create a special mechanism for these platform commands? For example via a top-level command "plat", i.e. instead of > rx_training params we would call > plat rx_training params The plat command could list all registered platform specific commands... Not sure. If you want to split it up, then we would perhaps also need to add a mechanism for board specific commands as well. E.g. for commands not common for a platform but only for specific boards. My feeling is that this makes things overly complex. And I also don't see a real problem with the current "flat" structure of these commands being "global". Again, I mention the many already existing board and platform specific commands in current mainline. Perhaps other people can comment on this use / introduction of platform specific U-Boot commands? BTW: Again, we can definitely rename this specific "rx_training" command, if you feel this is absolutely misleading. IIRC, then I already made a suggestion for this. Thanks, Stefan
Re: [PATCH v1 00/23] phy: marvell: Sync Armada 3k/7k/8k SERDES code with Marvell version
On Thu, 29 Apr 2021 08:46:36 +0200 Stefan Roese wrote: > >phy: marvell: add RX training command > > Applied to u-boot-marvell/master > > Thanks, > Stefan Stefan, do you think it would make sense to at least create a special mechanism for these platform commands? For example via a top-level command "plat", i.e. instead of > rx_training params we would call > plat rx_training params The plat command could list all registered platform specific commands... Marek
Re: [PATCH v1 00/23] phy: marvell: Sync Armada 3k/7k/8k SERDES code with Marvell version
On 24.03.21 15:06, Stefan Roese wrote: This patchset adds the missing SERDES patches from the Marvell U-Boot SDK U-Boot version. This is done in preparation for the integration of the Octeon TX2 CN913x support, which uses the updated version of this code. Thanks, Stefan Christine Gharzuzi (1): phy: marvell: fix handling of unconnected comphy Grzegorz Jaszczyk (8): phy: marvell: cp110: let the firmware configure comphy for RXAUI phy: marvell: cp110: let the firmware configure comphy for USB phy: marvell: cp110: let the firmware perform training for XFI phy: marvell: cp110: remove both phy and pipe selector configuration phy: marvell: cp110: clean up driver after it was moved to atf phy: marvell: allow to initialize up to 6 USB ports phy: marvell: fix pll initialization for second utmi port phy: marvell: utmi: update utmi config which fixes usb2.0 instability Igal Liberman (11): phy: marvell: rename comphy related definitions to COMPHY_XX phy: marvell: add missing speed during info prints phy: marvell: cp110: utmi: update analog parameters according to latest ETP phy: marvell: fix several minor bugs in comphy_probe phy: marvell: save comphy_map_data priv structure phy: marvell: add RX training command phy: marvell: enable comphy info prints for all devices phy: marvell: pass sgmii id to firmware phy: marvell: cp110: mark u-boot power-off calls phy: marvell: add support for SFI1 doc: dt-bindings: add Marvell comphy binding Marcin Wojtas (1): phy: marvell: cp110: remove unused definitions Omri Itach (1): phy: marvell: cp110: initialize only enabled UTMI units jinghua (1): phy: marvell: add comphy type PHY_TYPE_USB3 arch/arm/dts/armada-3720-db.dts | 8 +- arch/arm/dts/armada-3720-espressobin.dts | 12 +- arch/arm/dts/armada-3720-turris-mox.dts | 12 +- arch/arm/dts/armada-3720-uDPU.dts | 23 +- arch/arm/dts/armada-7040-db-nand.dts | 24 +- arch/arm/dts/armada-7040-db.dts | 23 +- arch/arm/dts/armada-8040-clearfog-gt-8k.dts | 32 +- arch/arm/dts/armada-8040-db.dts | 24 +- arch/arm/dts/armada-8040-mcbin.dts| 27 +- arch/arm/dts/armada-8040-puzzle-m801.dts | 32 +- arch/arm/dts/armada-cp110.dtsi| 36 +- arch/arm/dts/cn9130-crb-A.dts | 16 +- arch/arm/dts/cn9130-crb-B.dts | 16 +- board/CZ.NIC/turris_mox/turris_mox.c | 8 +- cmd/mvebu/Kconfig | 7 + cmd/mvebu/Makefile| 2 +- cmd/mvebu/rx_training.c | 57 ++ configs/mvebu_db_armada8k_defconfig | 1 + doc/device-tree-bindings/phy/mvebu_comphy.txt | 68 ++ drivers/phy/marvell/comphy_a3700.c| 70 +- drivers/phy/marvell/comphy_a3700.h| 1 - drivers/phy/marvell/comphy_core.c | 81 ++- drivers/phy/marvell/comphy_core.h | 67 +- drivers/phy/marvell/comphy_cp110.c| 621 drivers/phy/marvell/comphy_hpipe.h| 660 -- drivers/phy/marvell/comphy_mux.c | 11 +- drivers/phy/marvell/utmi_phy.h| 24 +- include/dt-bindings/comphy/comphy_data.h | 80 +-- include/mvebu/comphy.h| 2 +- 29 files changed, 591 insertions(+), 1454 deletions(-) create mode 100644 cmd/mvebu/rx_training.c create mode 100644 doc/device-tree-bindings/phy/mvebu_comphy.txt delete mode 100644 drivers/phy/marvell/comphy_hpipe.h Applied to u-boot-marvell/master Thanks, Stefan
