Re: [PATCH v2 1/9] dmaengine: st_fdma: Add STMicroelectronics FDMA DT binding documentation

[Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>]

On Tuesday 29 September 2015 14:42:15 Peter Griffin wrote:
> On Tue, 29 Sep 2015, Arnd Bergmann wrote:
> > On Tuesday 29 September 2015 13:11:55 Peter Griffin wrote:
> > > Does it? I didn't think it did.
> > > 
> > > Using the instance number as a DT property defers the decision over what firmware to
> > > load to the driver, which can choose whatever firmware name it wishes.
> > > 
> > > e.g. in v4.3 it could load xyz.elf, in v4.4 it could choose abc.elf. The DT will remain
> > > unchanged, but the use of that fdma instance has changed.
> > > 
> > > We currently only have one firmware for each instance with the "use" compiled into it.
> > > If in the future we had two firmwares with different "uses" for the same instance some extra
> > > logic would be required in the driver to make a decision on which firmware to load.
> > 
> > Ok, I probably need some more background about what the firmware on this
> > device does, and what it could do with a different firmware. Could you
> > elaborate?
> 
> So the fdma hw is a dma engine based around a xp70 slim core. It supports: -
> * block memory move between 2 system locations
> * paced transfer from system memory to paced peripheral
> * paced transfer from a paced peripheral to system memory
> 
> I believe each firmware for each instance supports those 3 things.

Ok

> However the xp70 also has some ancilary HW to facilitate Start Code Detection.
> It is this feature which I believe would require a different firmware if we wanted to make
> use of it. Looking at the functional spec each xp70 also
> has 16 gp output signals which we could also control from the firmware. Whether
> these are actually connected to anything useful inside the SoC I don't know.

I still don't understand what Start Code Detection is here.

> > > > and you correctly describe the problem with
> > > > using the compatible string for the firmware name if the driver for the FDMA
> > > > does not actually care what firmware is being used here.
> > > > 
> > > > Whatever code makes the decision as to how the FDMA is used should also
> > > > decide on the name of the firmware file.
> > > 
> > > The code which makes this decision currently is the st_fdma.c driver. However it does
> > > need to know which fdma controller it is operating on to make this decision correctly.
> > > 
> > > Apart from passing the fdma instance number in DT, how else can we determine which
> > > controller we are?
> > > 
> > > I guess we could infer it by having a table in the driver containing the base addresses
> > > of the controllers for a given SoC, and match that against what DT passes us in the
> > > reg property. But that seems ugly, and is encoding the same information in two
> > > different places.
> > > 
> > > I'm open to suggestions if there is a better way to do this.
> > 
> > Using the address would be the same thing, that doesn't change the
> > fundamental logic. Can you explain why it matters which instance
> > a firmware is used on for this driver?
> 
> The reason we care, is that each instance has its own firmware file.
> 
> I just did a hexdump on the 3 different firmwares and compared them. Although the majority
> of the binary is the same, there are various bytes which change at several different offsets
> in the firmware file depending on the instance.
> 
> I don't have a xp70 toolchain or know enough about the cpu architecture to analyze what exactly
> the firmware is doing at these locations.

This sounds like we should indeed treat them as different things: We don't
know what the difference is, but we know that each of them needs a different
firmware, and presumably if you get a new SoC variant, it will in turn
need three new ones.

In this case, I'd say using the compatible string to identify them is best,
using whatever the SoC documentation uses to describe them. You can use the
of_device_id data fields to look up the firmware name then.

If the output signals end up being connected to something we want to
control through the firmware, that also makes sense for a new compatible
string, as the driver needs to know about the feature in order to
communicate with the firmware, and the DT needs to be able to describe
the pins (e.g. by making the node act as a GPIO controller) in a way that
requires a different string, too.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html