Re: [v7 3/7] arm64: defconfig: Enable HIMAX_HX83102 panel

[Doug Anderson]

Hi,

On Wed, May 15, 2024 at 11:55 PM <neil.armstrong@xxxxxxxxxx> wrote:
>
> On 16/05/2024 08:43, cong yang wrote:
> > Hi:
> >
> > If it is determined that a separately patch needs to be sent, then I
> > will remove this patch in V8 series?
> >
> > Doug Anderson <dianders@xxxxxxxxxxxx> 于2024年5月16日周四 05:28写道：
> >
> >>
> >> Hi,
> >>
> >> On Wed, May 15, 2024 at 2:16 PM <neil.armstrong@xxxxxxxxxx> wrote:
> >>>
> >>> Hi,
> >>>
> >>> On 15/05/2024 03:46, Cong Yang wrote:
> >>>> DRM_PANEL_HIMAX_HX83102 is being split out from DRM_PANEL_BOE_TV101WUM_NL6.
> >>>> Since the arm64 defconfig had the BOE panel driver enabled, let's also
> >>>> enable the himax driver.
> >>>>
> >>>> Signed-off-by: Cong Yang <yangcong5@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> >>>> Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> >>>> ---
> >>>>    arch/arm64/configs/defconfig | 1 +
> >>>>    1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> >>>> index 2c30d617e180..687c86ddaece 100644
> >>>> --- a/arch/arm64/configs/defconfig
> >>>> +++ b/arch/arm64/configs/defconfig
> >>>> @@ -864,6 +864,7 @@ CONFIG_DRM_PANEL_BOE_TV101WUM_NL6=m
> >>>>    CONFIG_DRM_PANEL_LVDS=m
> >>>>    CONFIG_DRM_PANEL_SIMPLE=m
> >>>>    CONFIG_DRM_PANEL_EDP=m
> >>>> +CONFIG_DRM_PANEL_HIMAX_HX83102=m
> >>>>    CONFIG_DRM_PANEL_ILITEK_ILI9882T=m
> >>>>    CONFIG_DRM_PANEL_MANTIX_MLAF057WE51=m
> >>>>    CONFIG_DRM_PANEL_RAYDIUM_RM67191=m
> >>>
> >>> You should probably sent this one separately since only an ARM SoC maintainer
> >>> can apply this, probably via the qcom tree.
> >>
> >> Really? I always kinda figured that this was a bit like MAINTAINERS
> >> where it can come through a bunch of different trees. Certainly I've
> >> landed changes to it before through the drm-misc tree. If that was
> >> wrong then I'll certainly stop doing it, of course.
>
> Yeah we usually don't mess with arch specific defconfig from drm tree

In general I agree that makes sense. In this case, though, the new
config symbol was introduced in the previous patch and split off an
existing symbol. Updating "all" of the configs (AKA just arm64) that
had the old symbol to also have the new symbol seems like the nice
thing to do and it feels like it makes sense to land in the same tree
that did the "split" just to cause the least confusion to anyone
affected.

In any case, if it's going to land in some other tree then I guess the
question is whether it needs to wait a few revisions to land there or
if it should land right away. Nobody would get a compile error if it
landed in a different tree right away since unknown config symbols are
silently ignored, but it feels a little weird to me.

..of course, I'm also OK just dropping the config patch. I personally
don't use the upstream "defconfig". It just seemed courteous to update
it for those who do.

-Doug