[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Simon Glass]

Hi Rob,

On 19 July 2017 at 09:24, Rob Clark <robdclark at gmail.com> wrote:
> On Tue, Jul 18, 2017 at 11:06 AM, Alexander Graf <agraf at suse.de> wrote:
>> On 07/18/2017 04:54 PM, Simon Glass wrote:
>>>
>>> Hi Alex,
>>>
>>> On 18 July 2017 at 07:47, Alexander Graf <agraf at suse.de> wrote:
>>>>
>>>> On 07/18/2017 04:00 PM, Simon Glass wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 12 July 2017 at 05:52, Alexander Graf <agraf at suse.de> wrote:
>>>>>>
>>>>>>
>>>>>> On 25.06.17 01:05, Rob Clark wrote:
>>>>>>>
>>>>>>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>>>>>>> Cc: Alexander Graf <agraf at suse.de>
>>>>>>
>>>>>>
>>>>>> Looks reasonable to me, but could probably use a commit message ;).
>>>>>> Also
>>>>>> please make sure to CC Simon on all things DM.
>>>>>>
>>>>> Can we drop the CONFIG_LCD support entirely? This is legacy code at
>>>>> this point. What boards use it?
>>>>
>>>>
>>>> Sounds like someone would first need to convert a bunch of boards :).
>>>>
>>>> $ for i in $(grep CONFIG_LCD configs/* | cut -d : -f 1); do grep -q
>>>> DM_VIDEO
>>>> $i || echo $i; done
>>>> configs/at91sam9261ek_dataflash_cs0_defconfig
>>>> configs/at91sam9261ek_dataflash_cs3_defconfig
>>>> configs/at91sam9261ek_nandflash_defconfig
>>>> configs/at91sam9263ek_dataflash_cs0_defconfig
>>>> configs/at91sam9263ek_dataflash_defconfig
>>>> configs/at91sam9263ek_nandflash_defconfig
>>>> configs/at91sam9263ek_norflash_boot_defconfig
>>>> configs/at91sam9263ek_norflash_defconfig
>>>> configs/at91sam9g10ek_dataflash_cs0_defconfig
>>>> configs/at91sam9g10ek_dataflash_cs3_defconfig
>>>> configs/at91sam9g10ek_nandflash_defconfig
>>>> configs/at91sam9m10g45ek_mmc_defconfig
>>>> configs/at91sam9m10g45ek_nandflash_defconfig
>>>> configs/at91sam9n12ek_mmc_defconfig
>>>> configs/at91sam9n12ek_nandflash_defconfig
>>>> configs/at91sam9n12ek_spiflash_defconfig
>>>> configs/at91sam9rlek_dataflash_defconfig
>>>> configs/at91sam9rlek_mmc_defconfig
>>>> configs/at91sam9rlek_nandflash_defconfig
>>>> configs/at91sam9x5ek_dataflash_defconfig
>>>> configs/at91sam9x5ek_mmc_defconfig
>>>> configs/at91sam9x5ek_nandflash_defconfig
>>>> configs/at91sam9x5ek_spiflash_defconfig
>>>> configs/brppt1_mmc_defconfig
>>>> configs/brppt1_nand_defconfig
>>>> configs/brppt1_spi_defconfig
>>>> configs/brxre1_defconfig
>>>> configs/cm_t3517_defconfig
>>>> configs/cm_t35_defconfig
>>>> configs/picosam9g45_defconfig
>>>> configs/pm9261_defconfig
>>>> configs/pm9263_defconfig
>>>> configs/sama5d36ek_cmp_mmc_defconfig
>>>> configs/sama5d36ek_cmp_nandflash_defconfig
>>>> configs/sama5d36ek_cmp_spiflash_defconfig
>>>> configs/sama5d3xek_mmc_defconfig
>>>> configs/sama5d3xek_nandflash_defconfig
>>>> configs/sama5d3xek_spiflash_defconfig
>>>> configs/sama5d4ek_mmc_defconfig
>>>> configs/sama5d4ek_nandflash_defconfig
>>>> configs/sama5d4ek_spiflash_defconfig
>>>> configs/zipitz2_defconfig
>>>
>>> Not really. I suspect none of those uses EFI_LOADER
>>
>>
>> Why not? I really don't want to limit EFI_LOADER to something I consider
>> good. It's supposed to be the *one* interface that just works for everyone.
>>
>>> There is video driver for atmel which is most of the boards in that
>>> list, but we can disable EFI_LOADER until they are converted.
>>
>>
>> No, I won't disable EFI_LOADER on any board because it's not converted. I'd
>> rather add support to EFI_LOADER to support more boards that are not
>> converted to DM ;).
>>
>>> We should avoid adding new features to legacy code paths as it makes
>>> DM conversion harder and less likely to complete.
>>
>>
>> I agree, but the solution is not to disable EFI_LOADER, it's to convert
>> boards.
>>
>
> So what is the conclusion on this patch?  I can re-send with a commit
> msg (although there is not much to say beyond $subject)..  I kinda
> think we should merge this for now, unless dropping CONFIG_LCD is
> imminent in which case I can re-work it to drop the CONFIG_LCD and add
> CONFIG_DM_VIDEO.. EFI_LOADER should definitely support the non-legacy
> case and probably should not remove the legacy case until it does not
> exist anymore.

Yes I think resend with a commit message and apply it.

I'll look at a patch to make EFI_LOADER depend on DM, which I think
should have been done at the start. Supporting legacy code paths with
new features is just not a good idea.

>
> btw, I had a bit of time today to rebase my u-boot patches and start
> cleaning up some of the other patches I have for (re)posting.. so
> expect some more patches soon(ish)
>
> BR,
> -R

Regards,
Simon