Re: [PATCH 3/5] misc: Add Apple DART driver

From: Simon Glass <sjg@chromium.org>
To: Mark Kettenis <mark.kettenis@xs4all.nl>
Cc: Mark Kettenis <kettenis@openbsd.org>,
	U-Boot Mailing List <u-boot@lists.denx.de>,
	 Oliver Graute <oliver.graute@kococonnector.com>,
	Stephan Gerhold <stephan@gerhold.net>,
	 Masami Hiramatsu <masami.hiramatsu@linaro.org>,
	Priyanka Jain <priyanka.jain@nxp.com>,
	Leo Liang <ycliang@andestech.com>,
	Padmarao Begari <padmarao.begari@microchip.com>,
	 Michael Walle <michael@walle.cc>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	 Heinrich Schuchardt <xypron.glpk@gmx.de>,
	Bin Meng <bin.meng@windriver.com>,
	 Asherah Connor <ashe@kivikakk.ee>,
	Michal Simek <michal.simek@xilinx.com>,
	 Wasim Khan <wasim.khan@nxp.com>, Heiko Schocher <hs@denx.de>,
	 Igor Opaniuk <igor.opaniuk@foundries.io>, Ye Li <ye.li@nxp.com>,
	Stefan Roese <sr@denx.de>,
	AKASHI Takahiro <takahiro.akashi@linaro.org>,
	Weijie Gao <weijie.gao@mediatek.com>,
	 Vabhav Sharma <vabhav.sharma@nxp.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	 Pratyush Yadav <p.yadav@ti.com>
Subject: Re: [PATCH 3/5] misc: Add Apple DART driver
Date: Mon, 27 Sep 2021 14:14:54 -0600	[thread overview]
Message-ID: <CAPnjgZ0_nhMQOPZPCuMOi2eiprYgNJ0Gs3xNbWz+EeF_o5U5Rw@mail.gmail.com> (raw)
In-Reply-To: <561488006734eda7@bloch.sibelius.xs4all.nl>

Hi Mark,

On Sun, 26 Sept 2021 at 14:53, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > From: Simon Glass <sjg@chromium.org>
> > Date: Mon, 20 Sep 2021 19:11:25 -0600
>
> Hi Simon,
>
> > [..]
> >
> > > > > +static int apple_dart_bind(struct udevice *dev)
> > > > > +{
> > > > > +       void *base;
> > > > > +       int sid, i;
> > > > > +
> > > > > +       base = dev_read_addr_ptr(dev);
> > > > > +       if (!base)
> > > > > +               return -EINVAL;
> > > > > +
> > > > > +       u32 params2 = readl(base + DART_PARAMS2);
> > > > > +       if (params2 & DART_PARAMS2_BYPASS_SUPPORT) {
> > > > > +               for (sid = 0; sid < 16; sid++) {
> > > > > +                       writel(DART_TCR_BYPASS_DART | DART_TCR_BYPASS_DAPF,
> > > > > +                              base + DART_TCR(sid));
> > > > > +                       for (i = 0; i < 4; i++)
> > > > > +                               writel(0, base + DART_TTBR(sid, i));
> > > > > +               }
> > > > > +       }
> > > >
> > > > Not allowed hardware access in bind(). Can this more to probe() ?
> > >
> > > Well, I need to make sure that this happens before other drivers get
> > > probed (in particular the xhci-dwc3 driver).  Is there a better
> > > mechanism to achieve that?
> >
> > If those drivers have something in the DT indicating that they need
> > this, then you can add a uclass_get_device_by_phandle() in those
> > drivers.
>
> Yes, there is an "iommus" property that points at the IOMMU.  This is
> a standard property and I think it would make sense to have the core
> device probing code handle it in a way similar to how "power-domains"
> is handled.  That way we don't have to add IOMMU hooks into each and
> every driver.

OK.

>
> > If not, then you can probe all the DART devices with uclass_probe_all().
> >
> > Having said that, I see you are using UCLASS_MISC. I suspect this
> > should have its own UCLASS_IOMMU.
>
> So here is a proof-of-concept diff to add UCLASS_IOMMU.  This
> deliberately does not yet introduce driver ops yet; coming up with
> something sensible there needs a bit more thought.  For USB support
> all I need is for the probe function to put the IOMMU in bypass mode,
> so that is what's implemented here.
>
> Does this look reasonable?  I'll split this up eventually into a
> commit that adds UCLASS_IOMMU and a commit that adds the apple_dart
> driver.

Yes from a quick look. Remember for the IOMMU uclass to add a sandbox
test, even though for now it will only probe the driver since there
are no ops.

Also you don't need the #if in iommu-uclass.c as the linked will
garbage-collect it.

Regards,
Simon