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

[Rob Clark]

On Fri, Jul 21, 2017 at 11:58 AM, Tom Rini <trini at konsulko.com> wrote:
> On Fri, Jul 21, 2017 at 04:48:23AM -0600, Simon Glass wrote:
>> 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.
>
> Well, that's very much not how the EFI_LOADER maintainer wants go :)  I
> think I'd rather go with disabling the video side of the various boards,
> until they're converted.
>

removing just the legacy video part of things would clean up a small
bit of the mess.. but fixing the efi block devices (disks) stuff
depends somewhat on DM..  I'm leaving the legacy case in place (just
as broken as it currently is), but it is messy.

BR,
-R