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 <s...@denx.de> 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
