Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
In-Reply-To: <YV6HpM8NlO29UjAI@kroah.com>
References: <20211006193819.2654854-1-swboyd@chromium.org> <20211006193819.2654854-3-swboyd@chromium.org>
 <YV6HpM8NlO29UjAI@kroah.com>
From:   Stephen Boyd <swboyd@chromium.org>
User-Agent: alot/0.9.1
Date:   Thu, 7 Oct 2021 16:42:48 -0400
Message-ID: <CAE-0n51QPTT3TcgNJf2jDoXaRdxNnnyQY6--UqpNTp9ZKToqbw@mail.gmail.com>
Subject: Re: [PATCH v2 02/34] component: Introduce the aggregate bus_type
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org,
        Daniel Vetter <daniel.vetter@ffwll.ch>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Rob Clark <robdclark@gmail.com>,
        Russell King <rmk+kernel@arm.linux.org.uk>,
        Saravana Kannan <saravanak@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Quoting Greg Kroah-Hartman (2021-10-06 22:37:40)
> On Wed, Oct 06, 2021 at 12:37:47PM -0700, Stephen Boyd wrote:
> >
> > Let's make the component driver into an actual device driver that has
> > probe/remove/shutdown functions. The driver will only be bound to the
> > aggregate device once all component drivers have called component_add()
> > to indicate they're ready to assemble the aggregate driver. This allows
> > us to attach shutdown logic (and in the future runtime PM logic) to the
> > aggregate driver so that it runs the hooks in the correct order.
>
> Why are you creating a new bus type and not using the auxiliary bus
> instead?
>
> You have seen Documentation/driver-api/auxiliary_bus.rst, right?
>

Nope, but I read it now. Thanks for the pointer.

My read of it is that the auxiliary bus is a way to slice up a single IP
block into multiple devices and then have drivers attach to those
different "pieces" of the IP. It avoids polluting the platform bus with
devices that don't belong on the platform bus because they are sub
components of a larger IP block that sits on the platform bus.

The aggregate bus is solving the reverse problem. It is rounding up a
collection of IP blocks that live on some bus (platform, i2c, spi,
whatever) and presenting them as a single aggregate device (sound card,
display card, whatever) whenever all the component devices call
component_add(). For example, we don't want to do operations on the
entire display pipeline until all the devices that make up the display
are probed and drivers are attached. I suppose the aggregate_device in
this patch series has a 1:1 relationship with the drm class_type that
makes up /sys/class/drm/cardN but there's also a couple sound users and
a power_supply user so I don't know the equivalent there.

Long term, maybe all of this component code could be placed directly
into the driver core? That's probably even more invasive of a change but
I imagine we could make device links with component_add() as we're
already doing with these patches and then have driver core call some
class function pointer when all the links are probed. That would
handle the 'bind/probe' callback for the aggregate device but it won't
handle the component_bind_all() path where we call bind_component() for
each component device that makes up the aggregate device. Maybe we can
add even more devices for the components and then call probe there too.

Sorry that's a long-winded non-answer. I don't think they're solving the
same problem so using the same bus type looks wrong. We'd have to take
two different paths depending on what type of device it is (aggregate
vs. auxiliary) so there's not much of anything that is shared code-wise.