Hi,
On Tue, Dec 22, 2015 at 10:05:03AM +0100, Stefan Roese wrote:
> I've consolidated a bit of the Armada XP / 38x defines and
> Kconfig options just very recently. Please take a look at
> these two patches from yesterday:
>
> http://patchwork.ozlabs.org/patch/559579/
> http://patchwork.ozlabs.org/patch/559580/
>
> It would be great if you could base your DS414 support on
> top of these as well (sorry, I missed adding you on CC for these
> patches). It should fairly easy and make things clearer (e.g.
> CONFIG_ARMADA_XP only defined on AXP devices).
I see.
> I could also add a new git branch available, with the latest
> versions of all the mvebu related patches applied. Just let
> me know if this would help.
No problem, I just fetched the relevant patches from the list and
applied them locally.
[...]
> > SATA Support
> > ------------
> >
> > There is a Marvell 88SX7042 controller attached to PCIe which is
> > supported by Linux's sata_mv driver but sadly not U-Boot's sata_mv.
> > I'm not sure if extending the latter to support PCI devices is worth the
> > effort at all.
>
> Yes, this is probably not worth the effort.
What bothers me is that Synology's patched U-Boot supports it, so I
consider it a regression for users wanting to switch to mainline. And
since there is sata_mv in U-Boot now and Linux's sata_mv simply supports
both the platform device and the various PCI devices, one might think it
should be straightforward to add support to it. But Linux's sata_mv with
it's 4.5k LoC is quite a thing, too.
[...]
> > EHCI Support
> > ------------
> >
> > This seems functional after issuing 'usb start'. At least it detects USB
> > storage devices, and IIRC reading from them was OK. OTOH Linux fails to
> > register the controller if 'usb start' wasn't given before in U-Boot.
> >
> > According to Synology sources, this board seems to support USB device
> > (gadget?) mode. Though I didn't play around with it.
>
> This is something that should be fixed. Linux should be able to
> use all devices without any bootloader interference. So it might
> be, that some USB related configuration is missing here. Perhaps
> in the USB PHY (setup setup_usb_phys in cpu.c).
>
> Does this work in Linux when the original U-Boot is used?
I will check. Also printing the relevant registers in Linux and
comparing the values in working and broken condition might help.
> > PCIe Support
> > ------------
> >
> > This is fine, but trying to gate the clocks of unused lanes will hang
> > PCI enum. In addition to that, pci_mvebu seems not to support DM_PCI.
>
> Right. Patches welcome. ;)
I actually started, thought it can't be that hard. So far I'm stuck at a
point where enumeration gets just 0xfbfb for both vendor and product
IDs. Looks like a follow-up project to me. :)
[...]
> > diff --git a/arch/arm/mach-mvebu/serdes/axp/board_env_spec.h
> > b/arch/arm/mach-mvebu/serdes/axp/board_env_spec.h
> > index f00f327..3dca6a1 100644
> > --- a/arch/arm/mach-mvebu/serdes/axp/board_env_spec.h
> > +++ b/arch/arm/mach-mvebu/serdes/axp/board_env_spec.h
> > @@ -43,8 +43,9 @@
> > #define RD_78460_SERVER_REV2_ID (DB_78X60_PCAC_REV2_ID + 1)
> > #define DB_784MP_GP_ID (RD_78460_SERVER_REV2_ID + 1)
> > #define RD_78460_CUSTOMER_ID (DB_784MP_GP_ID + 1)
> > -#define MV_MAX_BOARD_ID (RD_78460_CUSTOMER_ID + 1)
> > -#define INVALID_BAORD_ID 0xFFFFFFFF
> > +#define SYNOLOGY_DS414_ID (RD_78460_CUSTOMER_ID + 1)
> > +#define MV_MAX_BOARD_ID (SYNOLOGY_DS414_ID + 1)
> > +#define INVALID_BOARD_ID 0xFFFFFFFF
>
> Do you really need these changes here?
Sadly, yes. See high_speed_env_lib.c for details: There it is needed by
serdes_phy_config() to get the right satr11 value via board_id_get().
Maybe this should be refactored to always use board_sat_r_get() and the
latter return a static value from a macro which board configs may define
instead of reading from i2c.
[...]
> > +/* DDR3 static MC configuration */
> > +
> > +/* 1G_v1 (4x2Gbits) adapted by DS414 */
> > +MV_DRAM_MC_INIT syno_ddr3_b0_667_1g_v1[MV_MAX_DDR3_STATIC_SIZE] = {
> > + {0x00001400, 0x73014A28}, /*DDR SDRAM Configuration Register */
> > + {0x00001404, 0x30000800}, /*Dunit Control Low Register */
> > + {0x00001408, 0x44148887}, /*DDR SDRAM Timing (Low) Register */
> > + /* {0x0000140C, 0x38000C6}, */ /*DDR SDRAM Timing (High) Register */
> > + {0x0000140C, 0x3AD83FEA}, /*DDR SDRAM Timing (High) Register */
>
> Why do you have commented-out values here? Its generally not allowed
> to add "dead code". So please either add it in some comment, so that
> it can be identified as a important comment. Or remove it completely.
Took that over from Synology's sources (mainly for the sake of
completeness). But you're right, without explanation (which I couldn't
find, either) it's pretty much useless.
[...]
> > + /* gate unused clocks */
> > + /* Note: disabling unused PCIe lanes will hang PCI bus scan */
> > + pwr_mng_ctrl_reg = reg_read(POWER_MNG_CTRL_REG);
> > + pwr_mng_ctrl_reg &= ~(BIT(0)); /* Audio */
> > + pwr_mng_ctrl_reg &= ~(BIT(1) | BIT(2)); /* GE3, GE2 */
> > + //pwr_mng_ctrl_reg &= ~(BIT(10) | BIT(11) | BIT(12)); /* PCIe11 -
> > PCIe13 */
>
> C++ style comments are not allowed, sorry. And again the same comment
> about dead code.
Ah, yes. Left it there in hopes to solve this PCI enum problem soon
(probably will be when DM_PCI works as that relieves us from doing full
bus scan), will put it into the note above instead.
[...]
> > +/*
> > + * High Level Configuration Options (easy to change)
> > + */
> > +#define CONFIG_ARMADA_XP /* SOC Family Name */
>
> This will not be needed any more with the 2 patches I reference
> above. The SoC is selected via Kconfig then.
OK.
> > +#define CONFIG_MV78230 /* SoC Version */
> > +#define CONFIG_SYNOLOGY_DS414 /* board target name for DDR
> > training and PCIe init */
I will review those as well. Maybe the CONFIG_ARMADA_XP solution could
be implemented for SoC types, too?
[...]
> > +/* Enable DDR support in SPL (DDR3 training from Marvell bin_hdr) */
> > +#define CONFIG_SYS_MVEBU_DDR_AXP
> > +#define MV_DDR_32BIT
>
> These 2 lines can also be removed with the new patches.
OK, CONFIG_SYS_MVEBU_DDR_AXP seems not to be used anymore at all. But
without MV_DDR_32BIT, BUS_WIDTH defaults to 64 which is wrong on DS414.
Thanks, Phil
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
