Re: [PATCH v2 7/7] arm64: dts: allwinner: a64: enable ANX6345 bridge on Teres-I

[Vasily Khoruzhick]

On Tue, Jul 9, 2019 at 1:55 AM Maxime Ripard <maxime.ripard@xxxxxxxxxxx> wrote:
>
> On Mon, Jul 08, 2019 at 05:49:21PM -0700, Vasily Khoruzhick wrote:
> > > > Maybe instead of edp-connector one would introduce integrator's specific
> > > > connector, for example with compatible "olimex,teres-edp-connector"
> > > > which should follow edp abstract connector rules? This will be at least
> > > > consistent with below presentation[1] - eDP requirements depends on
> > > > integrator. Then if olimex has standard way of dealing with panels
> > > > present in olimex/teres platforms the driver would then create
> > > > drm_panel/drm_connector/drm_bridge(?) according to these rules, I guess.
> > > > Anyway it still looks fishy for me :), maybe because I am not
> > > > familiarized with details of these platforms.
> > >
> > > That makes sense yes
> >
> > Actually, it makes no sense at all. Current implementation for anx6345
> > driver works fine as is with any panel specified assuming panel delays
> > are long enough for connected panel. It just doesn't use panel timings
> > from the driver. Creating a platform driver for connector itself looks
> > redundant since it can't be reused, it doesn't describe actual
> > hardware and it's just defeats purpose of DT by introducing
> > board-specific code.
>
> I'm not sure where you got the idea that the purpose of DT is to not
> have any board-specific code.

I believe DT was an attempt to move to declarative approach for
describing hardware. Yes, we have different compatibles for different
devices but they're specific to particular device rather than
particular board. Device interconnection is described in DT along with
some properties rather than in board-specific C-file. Introducing
board-specific compatible for a connector isn't looking right to me.

> It's perfectly fine to have some, that's even why there's a compatible
> assigned to each and every board.
>
> What the DT is about is allowing us to have a generic behaviour that
> we can detect: we can have a given behaviour for a given board, and a
> separate one for another one, and this will be evaluated at runtime.
>
> This is *exactly* what this is about: we can have a compatible that
> sets a given, more specific, behaviour (olimex,teres-edp-connector)
> while saying that this is compatible with the generic behaviour
> (edp-connector). That way, any OS will know what quirk to apply if
> needed, and if not that it can use the generic behaviour.
>
> And we could create a generic driver, for the generic behaviour if
> needed.
>
> > There's another issue: if we introduce edp-connector we'll have to
> > specify power up delays somewhere (in dts? or in platform driver?), so
> > edp-connector doesn't really solve the issue of multiple panels with
> > same motherboard.
>
> And that's what that compatible is about :)

Sorry, I fail to see how it would be different from using existing
panels infrastructure and different panels compatibles. I think Rob's
idea was to introduce generic edp-connector. If we can't make it
generic then let's use panel infrastructure.

> > I'd say DT overlays should be preferred solution here, not another
> > connector binding.
>
> Overlays are a way to apply a device tree dynamically. It's orthogonal
> to the binding.

It isn't orthogonal to original problem though.

> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com