[PATCH v4 06/12] arm: mvebu: clearfog: Add option for 2.5 Gbps SFP

[Joel Johnson]

On 2020-01-27 00:20, Baruch Siach wrote:
> Hi Joel,
> 
> On Mon, Jan 27 2020, Joel Johnson wrote:
>> On 2020-01-26 22:29, Baruch Siach wrote:
>>> On Mon, Jan 27 2020, Joel Johnson wrote:
>>>> While newer Linux kernels provide autoconfiguration of SFP, provide
>>>> an option for setting in U-Boot Kconfig for use prior to booting.
>>>> 
>>>> Signed-off-by: Joel Johnson <mrjoel at lixil.net>
>>>> 
>>>> ---
>>>> 
>>>> v2 changes:
>>>>   - fixed help indentation
>>>> v3 changes:
>>>>   - none
>>>> v4 changes:
>>>>   - adjust static SerDes configuration at runtime instead of #ifdef
>>>> 
>>>> ---
>>>>  board/solidrun/clearfog/Kconfig    | 7 +++++++
>>>>  board/solidrun/clearfog/clearfog.c | 3 +++
>>>>  2 files changed, 10 insertions(+)
>>>> 
>>>> diff --git a/board/solidrun/clearfog/Kconfig
>>>> b/board/solidrun/clearfog/Kconfig
>>>> index 936d5918f8..c910e17093 100644
>>>> --- a/board/solidrun/clearfog/Kconfig
>>>> +++ b/board/solidrun/clearfog/Kconfig
>>>> @@ -15,4 +15,11 @@ config TARGET_CLEARFOG_BASE
>>>>  	  detection via additional EEPROM hardware. This option enables
>>>> selecting
>>>>  	  the Base variant for older hardware revisions.
>>>> 
>>>> +config CLEARFOG_SFP_25GB
>>>> +	bool "Enable 2.5 Gbps mode for SFP"
>>>> +	help
>>>> +	  Set the SFP module connection to support 2.5 Gbps transfer speed 
>>>> for
>>>> the
>>>> +	  SGMII connection (requires a supporting SFP). By default, 
>>>> transfer
>>>> speed
>>>> +	  of 1.25 Gbps is used, suitable for a more common 1 Gbps SFP 
>>>> module.
>>>> +
>>>>  endmenu
>>>> diff --git a/board/solidrun/clearfog/clearfog.c
>>>> b/board/solidrun/clearfog/clearfog.c
>>>> index 4019e37016..6c5b9a784f 100644
>>>> --- a/board/solidrun/clearfog/clearfog.c
>>>> +++ b/board/solidrun/clearfog/clearfog.c
>>>> @@ -59,6 +59,9 @@ void config_static_serdes_map(void)
>>>>  		board_serdes_map[4].serdes_speed = SERDES_SPEED_5_GBPS;
>>>>  		board_serdes_map[4].serdes_mode = SERDES_DEFAULT_MODE;
>>>>  	}
>>>> +
>>>> +	if (IS_ENABLED(CONFIG_CLEARFOG_SFP_25GB))
>>>> +		board_serdes_map[5].serdes_speed = SERDES_SPEED_3_125_GBPS;
>>> 
>>> config_static_serdes_map() is not called only when TLV EEPROM is
>>> present. So CONFIG_CLEARFOG_SFP_25GB has no effect in that case. This
>>> code should move to hws_board_topology_load() to avoid that.
>> 
>> Yeah, that's what I was trying to ask with the question below embedded 
>> in
>> patch 4.
>> 
>>     "Note that this approach *does not* currently provide any 
>> mechanism
>>     for EEPROM detected boards to have their SFP speed changed or 
>> switch
>>     between PCIE/SATA signalling. I'm assuming that will be done based 
>> on
>>     hardware detection, but confirmation/acceptance in review would be
>>     appreciated so it can be revisited if needed.
>> 
>> After sending the series and thinking about it more, I didn't see any 
>> other
>> mechanism by which the configuration would be changed. The question 
>> was really
>> to confirm what I had inferred from code, namely that Pro/Base boards 
>> even
>> with EEPROM would still have to have a U-Boot built with custom 
>> options in
>> order to change the PCIe/SATA or SFP options. I would like to confirm 
>> with you
>> that is desired and expected even in the cases of EEPROM supporting 
>> units. If
>> that's the case, I'll rework the location of setting, along with 
>> updating the
>> help text for those options. Pending your confirmation, I plan to get 
>> rid of
>> the separate config_static_serdes_map function and fold it back in, 
>> with the
>> SATA and SFP options coming before the board runtime detection 
>> configuration.
> 
> My thinking is that EEPROM TLV is only meant to describe the hardware 
> as
> manufactured, and never change. SATA (read mSATA) and SFP are user 
> setup
> that TLV can't cover. So I think that SATA and SFP build time selection
> should apply regardless of TLV.
> 
> In theory it could have been even better to store SATA/SFP selection in
> environment variables. But that is not currently possible, since serdes
> configuration takes place at SPL phase where the environment is not
> accessible.
> 
> baruch

Yeah, that makes sense and as I thought further on it is the only 
logical option based at least on what's currently exposed for runtime 
detction. I'll send out the updated series with this change and a few 
code style cleanups after completing the SATA testing, hopefully later 
today.

Any other items pending for this series? I think it's in good shape, 
pending any further thoughts or discussion on the Pro DT by default 
change.

Joel