[U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option

[Nathan Rossi]

On Mon, Dec 14, 2015 at 5:29 PM, Michal Simek <michal.simek at xilinx.com> wrote:
> On 14.12.2015 07:26, Stefan Roese wrote:
>> On 12.12.2015 15:08, Nathan Rossi wrote:
>>> On Sat, Dec 12, 2015 at 10:05 PM, Stefan Roese <sr at denx.de> wrote:
>>>> On 11.12.2015 18:32, Marek Vasut wrote:
>>>>>
>>>>> On Friday, December 11, 2015 at 04:46:40 PM, Michal Simek wrote:
>>>>>>
>>>>>> On 11.12.2015 16:07, Marek Vasut wrote:
>>>>>>>
>>>>>>> On Friday, December 11, 2015 at 03:48:09 PM, Nathan Rossi wrote:
>>>>>>>>
>>>>>>>> The Device Model sequence alias feature is required by some
>>>>>>>> Uclasses.
>>>>>>>> Instead of disabling the feature for all SPL targets allow it to be
>>>>>>>> configured.
>>>>>>>>
>>>>>>>> The config option is disabled by default to reduce code size for
>>>>>>>> targets
>>>>>>>> that are not interested or do not require this feature.
>>>>>>>>
>>>>>>>> Signed-off-by: Nathan Rossi <nathan at nathanrossi.com>
>>>>>>>> Cc: Simon Glass <sjg at chromium.org>
>>>>>>>> Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
>>>>>>>> Cc: Linus Walleij <linus.walleij at linaro.org>
>>>>>>>> Cc: Marek Vasut <marex at denx.de>
>>>>>>>> Cc: Michal Simek <michal.simek at xilinx.com>
>>>>>>>> ---
>>>>>>>> Based on a small amount of inspection for the Zynq platform,
>>>>>>>> enabling
>>>>>>>> this config option adds ~1KB of code size.
>>>>>>>>
>>>>>>>> Also on a side note, this might affect the socfpga target as it
>>>>>>>> forcibly
>>>>>>>> overrides the #undef from config_uncmd_spl.h in its common header. I
>>>>>>>> have Cc'd the respective maintainer for this reason.
>>>>>>>
>>>>>>>
>>>>>>> The fix for SoCFPGA is easy -- enable the SPL_DM_SEQ_ALIAS in
>>>>>>> configs/socfpga*. It is needed for booting from QSPI NOR.
>>>
>>> I will add a patch to this series, depending on how it is agreed to
>>> enable this option.
>>>
>>>>>>
>>>>>>
>>>>>> That's probably not the best solution. But of course we can use it.
>>>>>> IRC Stefan had the same problem.
>>>
>>> So would a better solution be to force the ARCH_ZYNQ config (and
>>> others ARCH_SOCFPGA/ARCH_MVEBU) to 'select SPL_DM_SEQ_ALIAS' instead
>>> of enabling them separately for each defconfig? like is done for
>>> SPL_OF_CONTROL/etc.
>>
>> Yes, please. Autoselecting this option is definitely better.
>
> TBH I don't think this is the best solution. We should select things
> which are required all the time. For zynq case this selection is
> necessary just for qspi boot mode. There are other boot modes which
> doesn't require it that's why for these mods this is just additional code.
> If you think that this is need/good to select by default for your SoC
> then this can be done separately.

Sorry for the slow response/follow up on this.

So as I understand it makes sense for this to be configured slightly
differently depending on when the boot options that need this are
available. I guess this also makes sense as SPI is not the only user
of the seq alias feature.

I am sorting out v2 of this series for the additional patches to add
the config for socfpga and mvebu, just want to clarify that this would
be the preferred way (given recent comments) to enable it for the
corresponding platforms:
 * socfpga -> enable in each defconfig
 * mvebu -> enable in arch/arm/Kconfig for ARCH_MVEBU (aka auto selecting)

>
> Note: Agree with Marek that will be good to check where exactly the
> problem is that this config option is necessary to enable even for case
> where only on qspi is in the system.

I had a bit of a look into this, it would require changes to allow for
it to fall-back to finding and using the first device which is matched
for the uclass (instead of relying on just CONFIG_SF_DEFAULT_BUS), it
would probably be an intrusive change to sf-uclass. A simpler solution
might be to just default seq numbers to 0 when seq alias is not
enabled? would allow for the change to work with other uclasses that
use seq alias.

Regards,
Nathan

>
> Acked-by: Michal Simek <michal.simek at xilinx.com>
>
> Thanks,
> Michal