[PATCH u-boot-marvell 8/8] arm: mvebu: turris_omnia: Add support for USB3.0 mode in WWAN MiniPCIe slot

[Pali Rohár]

On Wednesday 02 March 2022 13:46:07 Marek Behún wrote:
> On Wed,  2 Mar 2022 12:47:58 +0100
> Pali Rohár <pali at kernel.org> wrote:
> 
> > PCIe Mini CEM 2.1 spec added support for USB3.0 mode on MiniPCIe cards.
> > USB3.0 and PCIe share same pins and only one function can be active at the
> > same time. PCIe Mini CEM 2.1 spec says that determining function is
> > platform specific and spec does not define any dedicated pin which could
> > say if card is USB3.0-based or PCIe-based.
> > 
> > Implement this platform specific decision (USB3.0 vs PCIe) for WWAN
> > MiniPCIe slot on Turris Omnia via U-Boot env variable "omnia_wwan_slot",
> > similarly like is implemented forced mode for MiniPCIe/mSATA slot via
> > "omnia_msata_slot" env variable. Value "usb3" for "omnia_wwan_slot" would
> > mean to set USB3.0 mode and value "pcie" original PCIe mode.
> > 
> > A385 SoC on Turris Omnia has configurable fifth SerDes line (exported to
> > MiniPCIe WWAN slot with SIM card) either to USB3.0 or PCIe functionality,
> > so implementation of this new PCIe Mini CEM 2.1 feature is simple, by just
> > configuring SerDes to USB 3.0 mode.
> > 
> > Other twos MiniPCIe slots on Turris Omnia do not have this new
> > functionality as their SerDes lines cannot be switched to USB3.0
> > functionality.
> > 
> > Note that A385 SoC does not have too many USB3.0 blocks, so activating
> > USB3.0 in MiniPCIe cause that one external USB3.0 USB-A port would loose
> > USB3.0 functionality and would be downgraded just to USB2.0.
> > 
> > By default this MiniPCIe WWAN slot is in PCIe mode, like before.
> > 
> > To set this MiniPCIe WWAN slot to USB3.0 mode, call U-Boot commands:
> > 
> >   => setenv omnia_wwan_slot usb3
> >   => saveenv
> >   => reset  
> > 
> > Signed-off-by: Pali Rohár <pali at kernel.org>
> > ---
> >  board/CZ.NIC/turris_omnia/turris_omnia.c | 57 ++++++++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> > 
> > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > index e2f4daa827ed..83cfc80d1930 100644
> > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > @@ -246,6 +246,22 @@ static bool omnia_detect_sata(const char *msata_slot)
> >  	return stsword & MSATA_IND_STSBIT ? true : false;
> >  }
> >  
> > +static bool omnia_detect_wwan_usb3(const char *wwan_slot)
> > +{
> > +	puts("WWAN slot configuration... ");
> > +
> > +	if (wwan_slot && strcmp(wwan_slot, "usb3") == 0) {
> > +		puts("USB3.0\n");
> > +		return true;
> > +	}
> > +
> > +	if (wwan_slot && strcmp(wwan_slot, "pcie") != 0)
> > +		printf("unsupported env value '%s', fallback to... ", wwan_slot);
> 
> If  I recall correctly, Linux' and U-Boot's code style (in contrast to
> TF-A) normally wants
>   if (expr)        instead of       if (expr != 0)
> and
>   if (!expr)       instead of       if (expr == 0)

I guess that this applies for functions which return boolean value. And
not for strcmp() function which is not failure expression. But instead
it returns comparison value.

> Sometimes in spite of this style it is better to use == or != operator,
> for example in cases where one wants to compare a character to multiple
> literals:
>   if (c == 0x13 || c == 0x12 || c == 0x00)
> 
> But for strcmp() and friends, we normaly just use
>   if (!strcmp())
> 
> This is just a nitpick, I don't mind that much. Just saying this so
> you are aware of this in the future...
> 
> 
> > +
> > +	puts("PCIe+USB2.0\n");
> > +	return false;
> > +}
> > +
> >  void *env_sf_get_env_addr(void)
> >  {
> >  	/* SPI Flash is mapped to address 0xD4000000 only in SPL */
> > @@ -276,6 +292,20 @@ int hws_board_topology_load(struct serdes_map **serdes_map_array, u8 *count)
> >  		board_serdes_map[0].serdes_mode = SERDES_DEFAULT_MODE;
> >  	}
> >  
> > +#ifdef CONFIG_SPL_ENV_SUPPORT
> > +	/* beware that env_get() returns static allocated memory */
> > +	env_value = has_env ? env_get("omnia_wwan_slot") : NULL;
> > +#endif
> 
> Use
>   if (IS_ENABLED(CONFIG_SPL_ENV_SUPPORT))
> instead of macro wherever possible, so that an error can be caught by
> the compiler even if it is in the disabled part of the code.
> 
> Marek