st_fdma: Firmware filename in DT?

st_fdma: Firmware filename in DT?

[Peter Griffin]

Hi Rob, Pawel, Mark, Ian and Kumar,

Quick question regarding this series here
https://lkml.org/lkml/2015/7/8/832. and the proposed
st,fw-name binding.

What are the rules with putting firmware names into DT?
Is it allowed? If yes, is it worth having a generic binding?

>From what I can see TI are using: -
    ti,pm-firmware in wkup_m3_rproc.c

and Freescale are using: -
    fsl,sdma-ram-script-name in imx-sdma.c

So other platforms seem to be putting firmware names into DT,
there are probably other examples.

Most other STi drivers keep the firmware name in the C code, which is
usually my first preference. However with fdma it is more complicated
as there are seperate firmware files for each instance of the IP block.

Currently also the same IP block "fdma_mpe31" is used on a number
of different SoC's but each firmware is named: -

 fdma_<SoC>_<instancenum>.elf

e.g. fdma_STiH407_0.elf

So currently all we need to provide is a different firmware name in DT
for the new SoC and the driver "just works".

Presumably the alternative would be to add a whole bunch of compatibles
in the driver for each SoC, where the only difference from a
functional point of view would be to help build the correct string for
the firmware filename. However I'm also then wondering what the best way
would be to find out the instance name of the IP.

We could do it by parsing the node name e.g. fdma0-audio, or by adding
a "instance" DT property to the node?

Any guidance you can provide on the recommend "most correct" method
from a DT point of view would be very greatfully received :-)

regards,

Peter.







--
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

Re: st_fdma: Firmware filename in DT?

[Rob Herring]

+devicetree-spec as a good question to separate from the fire hose.

On Thu, Sep 3, 2015 at 9:49 AM, Peter Griffin <peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Hi Rob, Pawel, Mark, Ian and Kumar,
>
> Quick question regarding this series here
> https://lkml.org/lkml/2015/7/8/832. and the proposed
> st,fw-name binding.

Thanks for looking at the bigger picture.

> What are the rules with putting firmware names into DT?

Whatever you can sneak in without DT maintainers noticing...

> Is it allowed?

They are already there as you have found, so yes. But should they be
allowed? Possibly. I'm not saying no, but do have some concerns.

> If yes, is it worth having a generic binding?

If we decide it is a good idea, then yes. It doesn't look that hard to
arrive at something common.

Strictly speaking, the firmware name is just an agreed upon name
between the kernel and userspace. So why tie that into DT? Would other
OS's use it or want something different? What if we started including
paths in the names like <soc>/<firmware file>? Now we are imposing a
directory structure on the OS filesystem.

We'd also need to consider firmware that is needed to boot. In that
case, we could include firmware binary directly in the dtb. I think
that has been done before, but not in any standard way. u-boot FIT
images happen to be DTBs containing mostly binary blobs.

> From what I can see TI are using: -
>     ti,pm-firmware in wkup_m3_rproc.c
>
> and Freescale are using: -
>     fsl,sdma-ram-script-name in imx-sdma.c
>
> So other platforms seem to be putting firmware names into DT,
> there are probably other examples.
>
> Most other STi drivers keep the firmware name in the C code, which is
> usually my first preference. However with fdma it is more complicated
> as there are seperate firmware files for each instance of the IP block.
>
> Currently also the same IP block "fdma_mpe31" is used on a number
> of different SoC's but each firmware is named: -
>
>  fdma_<SoC>_<instancenum>.elf
>
> e.g. fdma_STiH407_0.elf
>
> So currently all we need to provide is a different firmware name in DT
> for the new SoC and the driver "just works".
>
> Presumably the alternative would be to add a whole bunch of compatibles
> in the driver for each SoC, where the only difference from a
> functional point of view would be to help build the correct string for
> the firmware filename. However I'm also then wondering what the best way
> would be to find out the instance name of the IP.

If the name/path is Linux specific, then that is probably what we should do.

You could perhaps make a policy that firmware files be named by
compatible string. So rather than translating from matching compatible
to an arbitrary file name, you enforce file name is "<vendor>,<ip
block>.fw" or something. I know we don't have policy in the kernel,
but we already have it with hardcoded file names and search paths.

> We could do it by parsing the node name e.g. fdma0-audio, or by adding
> a "instance" DT property to the node?

Generally we try to avoid caring about node names. Having some index
or numbering also comes up which we also try to avoid. Generally, if
you care about which instance you use for something, then there is
some property you care about and should add.

Rob

Re: st_fdma: Firmware filename in DT?

[Lee Jones]

[...]

> > If yes, is it worth having a generic binding?
> 
> If we decide it is a good idea, then yes. It doesn't look that hard to
> arrive at something common.
> 
> Strictly speaking, the firmware name is just an agreed upon name
> between the kernel and userspace. So why tie that into DT? Would other
> OS's use it or want something different? What if we started including
> paths in the names like <soc>/<firmware file>? Now we are imposing a
> directory structure on the OS filesystem.

In Linux we have a standard place for firmwares, so the path should
not be required.  I certainly agree that DT is no place for absolute
paths or OS'isums.

[...]

> > Presumably the alternative would be to add a whole bunch of compatibles
> > in the driver for each SoC, where the only difference from a
> > functional point of view would be to help build the correct string for
> > the firmware filename. However I'm also then wondering what the best way
> > would be to find out the instance name of the IP.
> 
> If the name/path is Linux specific, then that is probably what we should do.
> 
> You could perhaps make a policy that firmware files be named by
> compatible string. So rather than translating from matching compatible
> to an arbitrary file name, you enforce file name is "<vendor>,<ip
> block>.fw" or something. I know we don't have policy in the kernel,
> but we already have it with hardcoded file names and search paths.

Absolutely not.  Firmwares have no direct link to DT or platforms that
run DT specifically.  They are carried by most platforms these days.
Insisting on firmwares using a DT compatible string format is way off.

If we flip it the other way round, some subsystems derive the firmware
name from the 'node name'.  For instance, our zeroth General Purpose
Co-Processor RemoteProc driver has a corresponding node called
'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
suffix and et voilà, we load file:

  lib/firmware/rproc-st231-gp0-fw

> > We could do it by parsing the node name e.g. fdma0-audio, or by adding
> > a "instance" DT property to the node?
> 
> Generally we try to avoid caring about node names. Having some index
> or numbering also comes up which we also try to avoid. Generally, if
> you care about which instance you use for something, then there is
> some property you care about and should add.

Right, the alternative is a property like the ones already used.
However, as these are becoming more prevalent I suggested
standardising the property to avoid all these vendor specific firmware
properties cluttering up the place.

  firmware = "firmwarename.fw";
OR
  firmware-name = "firmwarename.fw";

... seems appropriate.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: st_fdma: Firmware filename in DT?

[Peter Griffin]

Hi Rob,

Many thanks for your thoughts on the issue.

On Thu, 03 Sep 2015, Rob Herring wrote:

> +devicetree-spec as a good question to separate from the fire hose.
> 
> On Thu, Sep 3, 2015 at 9:49 AM, Peter Griffin <peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > Hi Rob, Pawel, Mark, Ian and Kumar,
> >
> > Quick question regarding this series here
> > https://lkml.org/lkml/2015/7/8/832. and the proposed
> > st,fw-name binding.
> 
> Thanks for looking at the bigger picture.
> 
> > What are the rules with putting firmware names into DT?
> 
> Whatever you can sneak in without DT maintainers noticing...

:) I appear to have fallen at the first hurdle!

> 
> > Is it allowed?
> 
> They are already there as you have found, so yes. But should they be
> allowed? Possibly. I'm not saying no, but do have some concerns.

I was hoping you would have a strong opinion either for or against...

> 
> > If yes, is it worth having a generic binding?
> 
> If we decide it is a good idea, then yes. It doesn't look that hard to
> arrive at something common.
> 
> Strictly speaking, the firmware name is just an agreed upon name
> between the kernel and userspace. So why tie that into DT? Would other
> OS's use it or want something different? What if we started including
> paths in the names like <soc>/<firmware file>? Now we are imposing a
> directory structure on the OS filesystem.

To answer a few of your questions in terms of STi chipsets..

For FDMA at least it seems reasonable to tie it into the DT, as like I said
the same IP block 'fdma_mpe31' is used on multiple SoC's, where the only
difference is location of the IP in the memory map, and the firmware filename
which is loaded.

I'm not 100% sure if we have different firmware files
for various SoC's just because they are statically linked or if there
are other differences in the firmware. It could be that some of the IP
differences are in a way abstracted away from Linux due to the firmware.
In reality it doesn't matter as as have no visibility of the firmware
source code, so it is essentially a black box from our perspective and from
Linux's PoV the IP is the same, and all the driver code which interacts
with the firmware is the same. 

Regarding other OS's, way back when Windows CE was a thing and ran on STi
SH4 chipsets, these same fdma firmwares were used. So they are in theory 
(at least in the past) used by other OS's. In reality these days AFAIK Linux
really is the only primary OS used on STi platforms, with other RTOS's
running on the co-processors.

Regarding paths we certainly don't require the pathname. I would propose
that is up to the primary OS, to figure out the pathnames to check once it
has been given the filename.

Certainly Linux does this currently, and it seems like a reasonable assumption
to make of the primary OS. Like you say I don't think we want to impose
directory structure on the OS filesystem, as Android, Windows CE etc will
all have different ideas about where these things should live.

> 
> We'd also need to consider firmware that is needed to boot. In that
> case, we could include firmware binary directly in the dtb. I think
> that has been done before, but not in any standard way. u-boot FIT
> images happen to be DTBs containing mostly binary blobs.

For STi we don't have any, or any that are required to boot are loaded
by primary and secondary bootloaders.

Are you proposing there should be a DT property which means the
firmware is actually in the DTB and can be pulled out at boot by the
kernel?

If so it sounds like it could be a useful feature for some platforms, but
not one which we require AFAIK.

> 
> > From what I can see TI are using: -
> >     ti,pm-firmware in wkup_m3_rproc.c
> >
> > and Freescale are using: -
> >     fsl,sdma-ram-script-name in imx-sdma.c
> >
> > So other platforms seem to be putting firmware names into DT,
> > there are probably other examples.
> >
> > Most other STi drivers keep the firmware name in the C code, which is
> > usually my first preference. However with fdma it is more complicated
> > as there are seperate firmware files for each instance of the IP block.
> >
> > Currently also the same IP block "fdma_mpe31" is used on a number
> > of different SoC's but each firmware is named: -
> >
> >  fdma_<SoC>_<instancenum>.elf
> >
> > e.g. fdma_STiH407_0.elf
> >
> > So currently all we need to provide is a different firmware name in DT
> > for the new SoC and the driver "just works".
> >
> > Presumably the alternative would be to add a whole bunch of compatibles
> > in the driver for each SoC, where the only difference from a
> > functional point of view would be to help build the correct string for
> > the firmware filename. However I'm also then wondering what the best way
> > would be to find out the instance name of the IP.
> 
> If the name/path is Linux specific, then that is probably what we should do.

We don't need a path, and the firmware name isn't Linux specific per se. However
I've written some patches which work fine avoiding the need to have the fw name in
DT. See end of this email for an example.

I'm inclined to go with this approach, as it is I believe uncontroversial and
doesn't require a new firmware DT binding.

> 
> You could perhaps make a policy that firmware files be named by
> compatible string. So rather than translating from matching compatible
> to an arbitrary file name, you enforce file name is "<vendor>,<ip
> block>.fw" or something. I know we don't have policy in the kernel,
> but we already have it with hardcoded file names and search paths.

Whilst being a neat solution, I don't think changing the firmware filenames
is the way to go.

I suspect it will be the same for other vendors in that firmwares come
from many various teams throughtout the company, and getting them all to
change to a DT centric naming scheme is not realistic.

IMHO we should treat the firmware as a unchangeable black box, and that
includes the filename it is shipped with. It is certinaly well outside 
my sphere of influence to change.

> > We could do it by parsing the node name e.g. fdma0-audio, or by adding
> > a "instance" DT property to the node?
> 
> Generally we try to avoid caring about node names. Having some index
> or numbering also comes up which we also try to avoid. Generally, if
> you care about which instance you use for something, then there is
> some property you care about and should add.

I can't see any other way of doing it other than adding a "st,fdma-id" property,
or parsing the node name. The fdma-id isn't arbitary it matches the controller
number in the datasheet. There are no other useful properties I can see
which we need to add (possibly due to any differences being abstracted
away into the firmware). I opted for "st,fdma-id" property, as parsing the node
name seems like it would be fragile. The DT node now looks like this: -

fdma0: fdma0-audio@8e20000 {
	compatible = "st,stih407-fdma-mpe31";
	reg = <0x8e20000 0x20000>;
	interrupts = <GIC_SPI 5 IRQ_TYPE_NONE>;
	dma-channels = <16>;
	#dma-cells = <3>;
	st,fdma-id = <0>;
	clocks = <&clk_s_c0_flexgen CLK_FDMA>,
		 <&clk_s_c0_flexgen CLK_EXT2F_A9>,
		 <&clk_s_c0_flexgen CLK_EXT2F_A9>,
		 <&clk_s_c0_flexgen CLK_EXT2F_A9>;
	clock-names = "fdma_slim",
		      "fdma_hi",
		      "fdma_low",
		      "fdma_ic";
};

I'm inclined to go with this "firmware name not in DT" approach, unless I get
a strong endorsement from someone such as yourself that having the name in DT is
acceptable.

regards,

Peter.

Re: st_fdma: Firmware filename in DT?

[Peter Griffin]

Hi Lee,

On Fri, 04 Sep 2015, Lee Jones wrote:

> [...]
> 
> > > If yes, is it worth having a generic binding?
> > 
> > If we decide it is a good idea, then yes. It doesn't look that hard to
> > arrive at something common.
> > 
> > Strictly speaking, the firmware name is just an agreed upon name
> > between the kernel and userspace. So why tie that into DT? Would other
> > OS's use it or want something different? What if we started including
> > paths in the names like <soc>/<firmware file>? Now we are imposing a
> > directory structure on the OS filesystem.
> 
> In Linux we have a standard place for firmwares, so the path should
> not be required.  I certainly agree that DT is no place for absolute
> paths or OS'isums.
> 
> [...]
> 
> > > Presumably the alternative would be to add a whole bunch of compatibles
> > > in the driver for each SoC, where the only difference from a
> > > functional point of view would be to help build the correct string for
> > > the firmware filename. However I'm also then wondering what the best way
> > > would be to find out the instance name of the IP.
> > 
> > If the name/path is Linux specific, then that is probably what we should do.
> > 
> > You could perhaps make a policy that firmware files be named by
> > compatible string. So rather than translating from matching compatible
> > to an arbitrary file name, you enforce file name is "<vendor>,<ip
> > block>.fw" or something. I know we don't have policy in the kernel,
> > but we already have it with hardcoded file names and search paths.
> 
> Absolutely not.  Firmwares have no direct link to DT or platforms that
> run DT specifically.  They are carried by most platforms these days.
> Insisting on firmwares using a DT compatible string format is way off.
> 
> If we flip it the other way round, some subsystems derive the firmware
> name from the 'node name'.  For instance, our zeroth General Purpose
> Co-Processor RemoteProc driver has a corresponding node called
> 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
> suffix and et voilà, we load file:
> 
>   lib/firmware/rproc-st231-gp0-fw

IMO deriving from the node name seems fragile. Also imposing a linux'ism
"rproc" prefix on the firmware name doesn't seem correct as the firmwares
can be shared across OS's. Although this is how remoteproc subsys core
is currently working. It seems a generic DT firmware binding would actually
be most useful for the remoteproc subsystem.

I guess I should also comment about this on the ST remoteproc thread.

I'm curious though as to how the ST remoteproc driver then loads the firmware,
as that name doesn't look like a video or audio firmware filename that I've
seen shipped from ST. IIRC Usually it is named something like

vid_firmware-stih407.elf or audio_firmware-bd-stih407.elf

IMO we should be treating the firmware as a blackbox, and for me that also
includes the filename it is shipped with. Linux drivers making up their own
firmware names based on the name of their subsystem doesn't seem like a
good way forward to me. Presumably to test this driver you have to rename the
firmwares locally on your machine?

> 
> > > We could do it by parsing the node name e.g. fdma0-audio, or by adding
> > > a "instance" DT property to the node?
> > 
> > Generally we try to avoid caring about node names. Having some index
> > or numbering also comes up which we also try to avoid. Generally, if
> > you care about which instance you use for something, then there is
> > some property you care about and should add.
> 
> Right, the alternative is a property like the ones already used.
> However, as these are becoming more prevalent I suggested
> standardising the property to avoid all these vendor specific firmware
> properties cluttering up the place.

I agree a proliferation of vendor specific firmware properties isn't
s good way forward.

> 
>   firmware = "firmwarename.fw";
> OR
>   firmware-name = "firmwarename.fw";
> 
> ... seems appropriate.

Either of those is fine with me.

regards,

Peter.

Re: st_fdma: Firmware filename in DT?

[Lee Jones]

On Fri, 04 Sep 2015, Peter Griffin wrote:
> On Fri, 04 Sep 2015, Lee Jones wrote:
> > [...]
> > 
> > > > If yes, is it worth having a generic binding?
> > > 
> > > If we decide it is a good idea, then yes. It doesn't look that hard to
> > > arrive at something common.
> > > 
> > > Strictly speaking, the firmware name is just an agreed upon name
> > > between the kernel and userspace. So why tie that into DT? Would other
> > > OS's use it or want something different? What if we started including
> > > paths in the names like <soc>/<firmware file>? Now we are imposing a
> > > directory structure on the OS filesystem.
> > 
> > In Linux we have a standard place for firmwares, so the path should
> > not be required.  I certainly agree that DT is no place for absolute
> > paths or OS'isums.
> > 
> > [...]
> > 
> > > > Presumably the alternative would be to add a whole bunch of compatibles
> > > > in the driver for each SoC, where the only difference from a
> > > > functional point of view would be to help build the correct string for
> > > > the firmware filename. However I'm also then wondering what the best way
> > > > would be to find out the instance name of the IP.
> > > 
> > > If the name/path is Linux specific, then that is probably what we should do.
> > > 
> > > You could perhaps make a policy that firmware files be named by
> > > compatible string. So rather than translating from matching compatible
> > > to an arbitrary file name, you enforce file name is "<vendor>,<ip
> > > block>.fw" or something. I know we don't have policy in the kernel,
> > > but we already have it with hardcoded file names and search paths.
> > 
> > Absolutely not.  Firmwares have no direct link to DT or platforms that
> > run DT specifically.  They are carried by most platforms these days.
> > Insisting on firmwares using a DT compatible string format is way off.
> > 
> > If we flip it the other way round, some subsystems derive the firmware
> > name from the 'node name'.  For instance, our zeroth General Purpose
> > Co-Processor RemoteProc driver has a corresponding node called
> > 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
> > suffix and et voilà, we load file:
> > 
> >   lib/firmware/rproc-st231-gp0-fw
> 
> IMO deriving from the node name seems fragile. Also imposing a linux'ism
> "rproc" prefix on the firmware name doesn't seem correct as the firmwares
> can be shared across OS's. Although this is how remoteproc subsys core
> is currently working. It seems a generic DT firmware binding would actually
> be most useful for the remoteproc subsystem.

The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
RProc driver is welcome to supply a different firmware name if it
desires.  This is where I think a generic 'firmware' property would be
of use.

> I guess I should also comment about this on the ST remoteproc thread.
> 
> I'm curious though as to how the ST remoteproc driver then loads the firmware,
> as that name doesn't look like a video or audio firmware filename that I've
> seen shipped from ST. IIRC Usually it is named something like
> 
> vid_firmware-stih407.elf or audio_firmware-bd-stih407.elf

This driver came direct from ST.  I'm using their conventions as a
default, although I would be happy to conduct some investigation into
how this works for releases and suggest a better interface.  The only
reason I mentioned it here was because this is how others are using
other SS which are similar in some way.  Same for the Firmware SS I
guess.

In my unbiased PoV, I'd much prefer the name was derived from a
predefined properly.

> IMO we should be treating the firmware as a blackbox, and for me that also
> includes the filename it is shipped with. Linux drivers making up their own
> firmware names based on the name of their subsystem doesn't seem like a
> good way forward to me.

I guess the driver author was just using the fall-back convention for
testing.  I'm pretty sure RemoteProc isn't being shipped in products
yet.

> Presumably to test this driver you have to rename the firmwares
> locally on your machine?

The firmware that was provided to conduct testing was already named as
expected by the driver, but yes, if we were to test the firmwares you
mentioned they would either have to be renamed, or those names would
have to be provided to the RemoteProc SS in a different way.

> > > > We could do it by parsing the node name e.g. fdma0-audio, or by adding
> > > > a "instance" DT property to the node?
> > > 
> > > Generally we try to avoid caring about node names. Having some index
> > > or numbering also comes up which we also try to avoid. Generally, if
> > > you care about which instance you use for something, then there is
> > > some property you care about and should add.
> > 
> > Right, the alternative is a property like the ones already used.
> > However, as these are becoming more prevalent I suggested
> > standardising the property to avoid all these vendor specific firmware
> > properties cluttering up the place.
> 
> I agree a proliferation of vendor specific firmware properties isn't
> s good way forward.
> 
> >   firmware = "firmwarename.fw";
> > OR
> >   firmware-name = "firmwarename.fw";
> > 
> > ... seems appropriate.
> 
> Either of those is fine with me.

Just need a DT nod and I'll happily code it up.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: st_fdma: Firmware filename in DT?

[Arnd Bergmann]

On Friday 04 September 2015, Lee Jones wrote:
> > > If we flip it the other way round, some subsystems derive the firmware
> > > name from the 'node name'.  For instance, our zeroth General Purpose
> > > Co-Processor RemoteProc driver has a corresponding node called
> > > 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
> > > suffix and et voilà, we load file:
> > > 
> > >   lib/firmware/rproc-st231-gp0-fw
> > 
> > IMO deriving from the node name seems fragile. Also imposing a linux'ism
> > "rproc" prefix on the firmware name doesn't seem correct as the firmwares
> > can be shared across OS's. Although this is how remoteproc subsys core
> > is currently working. It seems a generic DT firmware binding would actually
> > be most useful for the remoteproc subsystem.
> 
> The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
> RProc driver is welcome to supply a different firmware name if it
> desires.  This is where I think a generic 'firmware' property would be
> of use.

The firmware file name is agreed on between the device driver and the
file system, so encoding the linux driver name in it seems appropriate.

Generally speaking, I'd say a good policy would be to try basing
the firmware name on the "compatible" property strings. That property
already contains a hierarchical list of models, which makes it particularly
easy to have firmware files for specific models or those that are shared
across multiple variations if necessary. Just ask for the most specific
compatible string first and try the more specific compatible strings
(with an appropriate prefix and/or postfix added by the driver) until
a file is found.

	Arnd

Re: st_fdma: Firmware filename in DT?

[Arnd Bergmann]

On Friday 04 September 2015, Lee Jones wrote:
> > > If we flip it the other way round, some subsystems derive the firmware
> > > name from the 'node name'.  For instance, our zeroth General Purpose
> > > Co-Processor RemoteProc driver has a corresponding node called
> > > 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
> > > suffix and et voilà, we load file:
> > > 
> > >   lib/firmware/rproc-st231-gp0-fw
> > 
> > IMO deriving from the node name seems fragile. Also imposing a linux'ism
> > "rproc" prefix on the firmware name doesn't seem correct as the firmwares
> > can be shared across OS's. Although this is how remoteproc subsys core
> > is currently working. It seems a generic DT firmware binding would actually
> > be most useful for the remoteproc subsystem.
> 
> The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
> RProc driver is welcome to supply a different firmware name if it
> desires.  This is where I think a generic 'firmware' property would be
> of use.

The firmware file name is agreed on between the device driver and the
file system, so encoding the linux driver name in it seems appropriate.

Generally speaking, I'd say a good policy would be to try basing
the firmware name on the "compatible" property strings. That property
already contains a hierarchical list of models, which makes it particularly
easy to have firmware files for specific models or those that are shared
across multiple variations if necessary. Just ask for the most specific
compatible string first and try the more specific compatible strings
(with an appropriate prefix and/or postfix added by the driver) until
a file is found.

	Arnd

Re: st_fdma: Firmware filename in DT?

[Lee Jones]

On Fri, 04 Sep 2015, Arnd Bergmann wrote:

> On Friday 04 September 2015, Lee Jones wrote:
> > > > If we flip it the other way round, some subsystems derive the firmware
> > > > name from the 'node name'.  For instance, our zeroth General Purpose
> > > > Co-Processor RemoteProc driver has a corresponding node called
> > > > 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
> > > > suffix and et voilà, we load file:
> > > > 
> > > >   lib/firmware/rproc-st231-gp0-fw
> > > 
> > > IMO deriving from the node name seems fragile. Also imposing a linux'ism
> > > "rproc" prefix on the firmware name doesn't seem correct as the firmwares
> > > can be shared across OS's. Although this is how remoteproc subsys core
> > > is currently working. It seems a generic DT firmware binding would actually
> > > be most useful for the remoteproc subsystem.
> > 
> > The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
> > RProc driver is welcome to supply a different firmware name if it
> > desires.  This is where I think a generic 'firmware' property would be
> > of use.
> 
> The firmware file name is agreed on between the device driver and the
> file system, so encoding the linux driver name in it seems appropriate.
> 
> Generally speaking, I'd say a good policy would be to try basing
> the firmware name on the "compatible" property strings. That property
> already contains a hierarchical list of models, which makes it particularly
> easy to have firmware files for specific models or those that are shared
> across multiple variations if necessary. Just ask for the most specific
> compatible string first and try the more specific compatible strings
> (with an appropriate prefix and/or postfix added by the driver) until
> a file is found.

It depends what you mean by "basing the firmware name on the
\"compatible\" property" here.  If you mean actually renaming the
firmware binary file to match a driver's compatible string, that's
absolutely out of the question.  Firmwares are not only OS agnostic,
but are also independent of any H/W description language a particular
OS or platform might be using.  Using DT'isums to rename these
binaries is not logical.

However, if you mean simply match on compatible string and supply the
name from within the driver, that's closer to the mark (as then we can
at least keep it in-house [kernel]), but it's still not particularly
practical for the aforementioned reasons mentioned by Peter earlier.

Why not just create a new 'firmware' property?  Simples! [0]

[0] http://www.oxforddictionaries.com/definition/english/simples

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: st_fdma: Firmware filename in DT?

[Rob Herring]

On Fri, Sep 4, 2015 at 8:26 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Fri, 04 Sep 2015, Arnd Bergmann wrote:
>
>> On Friday 04 September 2015, Lee Jones wrote:
>> > > > If we flip it the other way round, some subsystems derive the firmware
>> > > > name from the 'node name'.  For instance, our zeroth General Purpose
>> > > > Co-Processor RemoteProc driver has a corresponding node called
>> > > > 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
>> > > > suffix and et voilà, we load file:
>> > > >
>> > > >   lib/firmware/rproc-st231-gp0-fw
>> > >
>> > > IMO deriving from the node name seems fragile. Also imposing a linux'ism
>> > > "rproc" prefix on the firmware name doesn't seem correct as the firmwares
>> > > can be shared across OS's. Although this is how remoteproc subsys core
>> > > is currently working. It seems a generic DT firmware binding would actually
>> > > be most useful for the remoteproc subsystem.
>> >
>> > The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
>> > RProc driver is welcome to supply a different firmware name if it
>> > desires.  This is where I think a generic 'firmware' property would be
>> > of use.
>>
>> The firmware file name is agreed on between the device driver and the
>> file system, so encoding the linux driver name in it seems appropriate.
>>
>> Generally speaking, I'd say a good policy would be to try basing
>> the firmware name on the "compatible" property strings. That property
>> already contains a hierarchical list of models, which makes it particularly
>> easy to have firmware files for specific models or those that are shared
>> across multiple variations if necessary. Just ask for the most specific
>> compatible string first and try the more specific compatible strings
>> (with an appropriate prefix and/or postfix added by the driver) until
>> a file is found.
>
> It depends what you mean by "basing the firmware name on the
> \"compatible\" property" here.  If you mean actually renaming the
> firmware binary file to match a driver's compatible string, that's
> absolutely out of the question.  Firmwares are not only OS agnostic,
> but are also independent of any H/W description language a particular
> OS or platform might be using.  Using DT'isums to rename these
> binaries is not logical.
>
> However, if you mean simply match on compatible string and supply the
> name from within the driver, that's closer to the mark (as then we can
> at least keep it in-house [kernel]), but it's still not particularly
> practical for the aforementioned reasons mentioned by Peter earlier.
>
> Why not just create a new 'firmware' property?  Simples! [0]

Someone give me some evidence that other OS's use or will use the same
names. Does *BSD use linux-firmware would be enough. With the
complaints I get that bindings are just Linux driver properties, I'm
not inclined to take this. Having a filename does imply the OS has a
filesystem and drivers can access the filesystem which may not always
be true.

Rob

Re: st_fdma: Firmware filename in DT?

[Rob Herring]

On Fri, Sep 4, 2015 at 8:26 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Fri, 04 Sep 2015, Arnd Bergmann wrote:
>
>> On Friday 04 September 2015, Lee Jones wrote:
>> > > > If we flip it the other way round, some subsystems derive the firmware
>> > > > name from the 'node name'.  For instance, our zeroth General Purpose
>> > > > Co-Processor RemoteProc driver has a corresponding node called
>> > > > 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
>> > > > suffix and et voilà, we load file:
>> > > >
>> > > >   lib/firmware/rproc-st231-gp0-fw
>> > >
>> > > IMO deriving from the node name seems fragile. Also imposing a linux'ism
>> > > "rproc" prefix on the firmware name doesn't seem correct as the firmwares
>> > > can be shared across OS's. Although this is how remoteproc subsys core
>> > > is currently working. It seems a generic DT firmware binding would actually
>> > > be most useful for the remoteproc subsystem.
>> >
>> > The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
>> > RProc driver is welcome to supply a different firmware name if it
>> > desires.  This is where I think a generic 'firmware' property would be
>> > of use.
>>
>> The firmware file name is agreed on between the device driver and the
>> file system, so encoding the linux driver name in it seems appropriate.
>>
>> Generally speaking, I'd say a good policy would be to try basing
>> the firmware name on the "compatible" property strings. That property
>> already contains a hierarchical list of models, which makes it particularly
>> easy to have firmware files for specific models or those that are shared
>> across multiple variations if necessary. Just ask for the most specific
>> compatible string first and try the more specific compatible strings
>> (with an appropriate prefix and/or postfix added by the driver) until
>> a file is found.
>
> It depends what you mean by "basing the firmware name on the
> \"compatible\" property" here.  If you mean actually renaming the
> firmware binary file to match a driver's compatible string, that's
> absolutely out of the question.  Firmwares are not only OS agnostic,
> but are also independent of any H/W description language a particular
> OS or platform might be using.  Using DT'isums to rename these
> binaries is not logical.
>
> However, if you mean simply match on compatible string and supply the
> name from within the driver, that's closer to the mark (as then we can
> at least keep it in-house [kernel]), but it's still not particularly
> practical for the aforementioned reasons mentioned by Peter earlier.
>
> Why not just create a new 'firmware' property?  Simples! [0]

Someone give me some evidence that other OS's use or will use the same
names. Does *BSD use linux-firmware would be enough. With the
complaints I get that bindings are just Linux driver properties, I'm
not inclined to take this. Having a filename does imply the OS has a
filesystem and drivers can access the filesystem which may not always
be true.

Rob

Re: st_fdma: Firmware filename in DT?

[Lee Jones]

On Fri, 04 Sep 2015, Rob Herring wrote:

> On Fri, Sep 4, 2015 at 8:26 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Fri, 04 Sep 2015, Arnd Bergmann wrote:
> >
> >> On Friday 04 September 2015, Lee Jones wrote:
> >> > > > If we flip it the other way round, some subsystems derive the firmware
> >> > > > name from the 'node name'.  For instance, our zeroth General Purpose
> >> > > > Co-Processor RemoteProc driver has a corresponding node called
> >> > > > 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
> >> > > > suffix and et voilà, we load file:
> >> > > >
> >> > > >   lib/firmware/rproc-st231-gp0-fw
> >> > >
> >> > > IMO deriving from the node name seems fragile. Also imposing a linux'ism
> >> > > "rproc" prefix on the firmware name doesn't seem correct as the firmwares
> >> > > can be shared across OS's. Although this is how remoteproc subsys core
> >> > > is currently working. It seems a generic DT firmware binding would actually
> >> > > be most useful for the remoteproc subsystem.
> >> >
> >> > The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
> >> > RProc driver is welcome to supply a different firmware name if it
> >> > desires.  This is where I think a generic 'firmware' property would be
> >> > of use.
> >>
> >> The firmware file name is agreed on between the device driver and the
> >> file system, so encoding the linux driver name in it seems appropriate.
> >>
> >> Generally speaking, I'd say a good policy would be to try basing
> >> the firmware name on the "compatible" property strings. That property
> >> already contains a hierarchical list of models, which makes it particularly
> >> easy to have firmware files for specific models or those that are shared
> >> across multiple variations if necessary. Just ask for the most specific
> >> compatible string first and try the more specific compatible strings
> >> (with an appropriate prefix and/or postfix added by the driver) until
> >> a file is found.
> >
> > It depends what you mean by "basing the firmware name on the
> > \"compatible\" property" here.  If you mean actually renaming the
> > firmware binary file to match a driver's compatible string, that's
> > absolutely out of the question.  Firmwares are not only OS agnostic,
> > but are also independent of any H/W description language a particular
> > OS or platform might be using.  Using DT'isums to rename these
> > binaries is not logical.
> >
> > However, if you mean simply match on compatible string and supply the
> > name from within the driver, that's closer to the mark (as then we can
> > at least keep it in-house [kernel]), but it's still not particularly
> > practical for the aforementioned reasons mentioned by Peter earlier.
> >
> > Why not just create a new 'firmware' property?  Simples! [0]
> 
> Someone give me some evidence that other OS's use or will use the same
> names. Does *BSD use linux-firmware would be enough. With the
> complaints I get that bindings are just Linux driver properties, I'm
> not inclined to take this. Having a filename does imply the OS has a
> filesystem and drivers can access the filesystem which may not always
> be true.

Peter already provided a real-world example of multiple OSes using the
same Firmware based on his experience at ST.  Any vendor using this API
who doesn't _soley_ use Linux is likely to use the same firmware files
across all platforms.  The co-processor's jobs don't often change just
because the platform is running a different OS.

I don't know enough about the inner workings of other Operating
Systems to comment on your final statement, but if they can get
access to the filesystem them they'll need the name too.  If they
don't have access to the firmware file, then they don't require the
property and it becomes unused by them.  That's not an issue is it?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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

Re: st_fdma: Firmware filename in DT?

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2376 bytes --]


> On Sep 4, 2015, at 4:21 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> 
>>>>> We could do it by parsing the node name e.g. fdma0-audio, or by adding
>>>>> a "instance" DT property to the node?
>>>> 
>>>> Generally we try to avoid caring about node names. Having some index
>>>> or numbering also comes up which we also try to avoid. Generally, if
>>>> you care about which instance you use for something, then there is
>>>> some property you care about and should add.
>>> 
>>> Right, the alternative is a property like the ones already used.
>>> However, as these are becoming more prevalent I suggested
>>> standardising the property to avoid all these vendor specific firmware
>>> properties cluttering up the place.
>> 
>> I agree a proliferation of vendor specific firmware properties isn't
>> s good way forward.
>> 
>>>  firmware = "firmwarename.fw";
>>> OR
>>>  firmware-name = "firmwarename.fw";
>>> 
>>> ... seems appropriate.
>> 
>> Either of those is fine with me.
> 
> Just need a DT nod and I'll happily code it up.

From a FreeBSD perspective, having a filename for the firmware to
load for this node is fine. It doesn’t impose a substantial burden on
the OS so long as the choices of where that file lives is up to the OS
and not encoded in the property. A note saying ‘this firmware should
be loaded’ seems a reasonable description of the hardware since
the DT tells us many things about the device, and those things may
well be dependent on which firmware is loaded. Having an explicit
name is good since it helps insulate from DT and doesn’t force vendors
to do silly renames. It also allows for multiple devices of the same
type to have different firmware loaded.

FreeBSD would generally put these things in /boot/firmware and
it has a generalized mechanism to load the firmware at run time
that’s based on this. While the path names are flexible, having
the firmware live in a central place is useful from an automation
point of view. Having just a name, and not a full path, enables
this policy, while still allowing others to have other policies.

Linux distributions would be free to do whatever they wanted
and implement other policies than FreeBSD.

So a property like this, with the semantics discussed, seems to
meet the OS independent test.

Warner


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 842 bytes --]

Re: st_fdma: Firmware filename in DT?

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2546 bytes --]


> On Sep 4, 2015, at 7:04 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> 
> On Friday 04 September 2015, Lee Jones wrote:
>>>> If we flip it the other way round, some subsystems derive the firmware
>>>> name from the 'node name'.  For instance, our zeroth General Purpose
>>>> Co-Processor RemoteProc driver has a corresponding node called
>>>> 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
>>>> suffix and et voilà, we load file:
>>>> 
>>>>  lib/firmware/rproc-st231-gp0-fw
>>> 
>>> IMO deriving from the node name seems fragile. Also imposing a linux'ism
>>> "rproc" prefix on the firmware name doesn't seem correct as the firmwares
>>> can be shared across OS's. Although this is how remoteproc subsys core
>>> is currently working. It seems a generic DT firmware binding would actually
>>> be most useful for the remoteproc subsystem.
>> 
>> The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
>> RProc driver is welcome to supply a different firmware name if it
>> desires.  This is where I think a generic 'firmware' property would be
>> of use.
> 
> The firmware file name is agreed on between the device driver and the
> file system, so encoding the linux driver name in it seems appropriate.

Encoding the driver name is not OS independent. It fosters the
impression that DT isn’t OS independent, but rather some silly
Linux toy and hurts wider adaptation.

> Generally speaking, I'd say a good policy would be to try basing
> the firmware name on the "compatible" property strings. That property
> already contains a hierarchical list of models, which makes it particularly
> easy to have firmware files for specific models or those that are shared
> across multiple variations if necessary. Just ask for the most specific
> compatible string first and try the more specific compatible strings
> (with an appropriate prefix and/or postfix added by the driver) until
> a file is found.

I think this is a horrible policy. It is an ugly kludge that is fragile to change.
While it sounds cool, I don’t think it is really a viable one. It requires
different compatibility names for different bits of hardware that might otherwise
be the same just to get different firmware. Sometimes this may be OK,
but it does seem needlessly limiting for systems that may have firmware
“images” that are loaded into one hardware block, but actually control
other blocks indirectly (where the different compat strings would be).

Warner


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 842 bytes --]

Re: st_fdma: Firmware filename in DT?

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 5187 bytes --]


> On Sep 4, 2015, at 7:54 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> 
> On Fri, 04 Sep 2015, Rob Herring wrote:
> 
>> On Fri, Sep 4, 2015 at 8:26 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>> On Fri, 04 Sep 2015, Arnd Bergmann wrote:
>>> 
>>>> On Friday 04 September 2015, Lee Jones wrote:
>>>>>>> If we flip it the other way round, some subsystems derive the firmware
>>>>>>> name from the 'node name'.  For instance, our zeroth General Purpose
>>>>>>> Co-Processor RemoteProc driver has a corresponding node called
>>>>>>> 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
>>>>>>> suffix and et voilà, we load file:
>>>>>>> 
>>>>>>>  lib/firmware/rproc-st231-gp0-fw
>>>>>> 
>>>>>> IMO deriving from the node name seems fragile. Also imposing a linux'ism
>>>>>> "rproc" prefix on the firmware name doesn't seem correct as the firmwares
>>>>>> can be shared across OS's. Although this is how remoteproc subsys core
>>>>>> is currently working. It seems a generic DT firmware binding would actually
>>>>>> be most useful for the remoteproc subsystem.
>>>>> 
>>>>> The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
>>>>> RProc driver is welcome to supply a different firmware name if it
>>>>> desires.  This is where I think a generic 'firmware' property would be
>>>>> of use.
>>>> 
>>>> The firmware file name is agreed on between the device driver and the
>>>> file system, so encoding the linux driver name in it seems appropriate.
>>>> 
>>>> Generally speaking, I'd say a good policy would be to try basing
>>>> the firmware name on the "compatible" property strings. That property
>>>> already contains a hierarchical list of models, which makes it particularly
>>>> easy to have firmware files for specific models or those that are shared
>>>> across multiple variations if necessary. Just ask for the most specific
>>>> compatible string first and try the more specific compatible strings
>>>> (with an appropriate prefix and/or postfix added by the driver) until
>>>> a file is found.
>>> 
>>> It depends what you mean by "basing the firmware name on the
>>> \"compatible\" property" here.  If you mean actually renaming the
>>> firmware binary file to match a driver's compatible string, that's
>>> absolutely out of the question.  Firmwares are not only OS agnostic,
>>> but are also independent of any H/W description language a particular
>>> OS or platform might be using.  Using DT'isums to rename these
>>> binaries is not logical.
>>> 
>>> However, if you mean simply match on compatible string and supply the
>>> name from within the driver, that's closer to the mark (as then we can
>>> at least keep it in-house [kernel]), but it's still not particularly
>>> practical for the aforementioned reasons mentioned by Peter earlier.
>>> 
>>> Why not just create a new 'firmware' property?  Simples! [0]
>> 
>> Someone give me some evidence that other OS's use or will use the same
>> names. Does *BSD use linux-firmware would be enough. With the
>> complaints I get that bindings are just Linux driver properties, I'm
>> not inclined to take this. Having a filename does imply the OS has a
>> filesystem and drivers can access the filesystem which may not always
>> be true.
> 
> Peter already provided a real-world example of multiple OSes using the
> same Firmware based on his experience at ST.  Any vendor using this API
> who doesn't _soley_ use Linux is likely to use the same firmware files
> across all platforms.  The co-processor's jobs don't often change just
> because the platform is running a different OS.
> 
> I don't know enough about the inner workings of other Operating
> Systems to comment on your final statement, but if they can get
> access to the filesystem them they'll need the name too.  If they
> don't have access to the firmware file, then they don't require the
> property and it becomes unused by them.  That's not an issue is it?

Based on my experience with FreeBSD and different firmware and
such, I’d have to agree with Peter. FreeBSD often uses the vendor
supplied firmware, just like Linux, because that firmware establishes
the ABI for talking to the device. This ABI is typically OS agnostic,
and when it isn’t that’s more the exception than the rule.

As for ‘not all OSes provide a filesystem’ Sure, but who cares. While
the filesystem provides a natural mapping of the property to the
binary data to load (e.g. os-firmware-path + “/“ + property-name), that’s
not the only way to map names to binary blobs. FreeBSD itself allows
one to compile firmware images into the kernel, and those images
are addressed / found by name, even though no filesystem is involved
based on what the driver desires to load. I would imagine that an OS
minimal enough to not support a filesystem would run into this sort
of issue often enough to provide a mechanism similar to FreeBSD. Failing
that, I’d imagine driver writers would compile images into their
drivers on such a system and use the name to pick which one to load.

Warner

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 842 bytes --]

Re: st_fdma: Firmware filename in DT?

[Daniel Thompson]

On 04/09/15 11:21, Lee Jones wrote:
>>> Absolutely not.  Firmwares have no direct link to DT or platforms that
>>> run DT specifically.  They are carried by most platforms these days.
>>> Insisting on firmwares using a DT compatible string format is way off.
>>>
>>> If we flip it the other way round, some subsystems derive the firmware
>>> name from the 'node name'.  For instance, our zeroth General Purpose
>>> Co-Processor RemoteProc driver has a corresponding node called
>>> 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
>>> suffix and et voilà, we load file:
>>>
>>>    lib/firmware/rproc-st231-gp0-fw
>>
>> IMO deriving from the node name seems fragile. Also imposing a linux'ism
>> "rproc" prefix on the firmware name doesn't seem correct as the firmwares
>> can be shared across OS's. Although this is how remoteproc subsys core
>> is currently working. It seems a generic DT firmware binding would actually
>> be most useful for the remoteproc subsystem.
>
> The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
> RProc driver is welcome to supply a different firmware name if it
> desires.  This is where I think a generic 'firmware' property would be
> of use.

Hmnnn... That default looks really inappropriate for the ST platforms!

Different generations of the hardware have co-processors with similar 
roles (and hence all with names like st231-gp0) but its very rare for 
the co-processors to have the same firmware binary.

Thus if the default were applied naively we will end up with a bizarre 
situation where the kernel is multi-platform but because the kernel does 
not select a unique name for the different firmwares, it becomes 
needlessly difficult for the userspace to support multiple platforms.

If there was a generic firmware property for this kind of situation it 
is fairly natural to note in the bindings docs the importance of 
uniqueness through the generations regarding of the choice of name.

There are other ways a driver can generate a unique name as the 
generations of chip develop (compatible strings actually very good for 
this) but I don't see it fitting quite so naturally into the binding 
docs where it can help future adventurers.


>> I guess I should also comment about this on the ST remoteproc thread.
>>
>> I'm curious though as to how the ST remoteproc driver then loads the firmware,
>> as that name doesn't look like a video or audio firmware filename that I've
>> seen shipped from ST. IIRC Usually it is named something like
>>
>> vid_firmware-stih407.elf or audio_firmware-bd-stih407.elf
>
> This driver came direct from ST.  I'm using their conventions as a
> default, although I would be happy to conduct some investigation into
> how this works for releases and suggest a better interface.  The only
> reason I mentioned it here was because this is how others are using
> other SS which are similar in some way.  Same for the Firmware SS I
> guess.
>
> In my unbiased PoV, I'd much prefer the name was derived from a
> predefined propertly.
>
>> IMO we should be treating the firmware as a blackbox, and for me that also
>> includes the filename it is shipped with. Linux drivers making up their own
>> firmware names based on the name of their subsystem doesn't seem like a
>> good way forward to me.
>
> I guess the driver author was just using the fall-back convention for
> testing.  I'm pretty sure RemoteProc isn't being shipped in products
> yet.

Both for fdma and co-processors, it is important that the kernel (or 
kernel informed by DT properties/compatible strings) is "assertive" 
enough to choose names that are sufficiently unique.

That is far more important than honoring vendor filenames.

Vendor filenames could easily be both too simple ("firmware.bin") or too 
detailed ("fdma-foo+bar-v1.8_20140811.fw"; the DT should not be encoding 
the version number of a firmware loaded from userspace since its not a 
fixed property of the platform).


Daniel.

Re: st_fdma: Firmware filename in DT?

[Daniel Thompson]

On 04/09/15 11:21, Lee Jones wrote:
>>> Absolutely not.  Firmwares have no direct link to DT or platforms that
>>> run DT specifically.  They are carried by most platforms these days.
>>> Insisting on firmwares using a DT compatible string format is way off.
>>>
>>> If we flip it the other way round, some subsystems derive the firmware
>>> name from the 'node name'.  For instance, our zeroth General Purpose
>>> Co-Processor RemoteProc driver has a corresponding node called
>>> 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
>>> suffix and et voilà, we load file:
>>>
>>>    lib/firmware/rproc-st231-gp0-fw
>>
>> IMO deriving from the node name seems fragile. Also imposing a linux'ism
>> "rproc" prefix on the firmware name doesn't seem correct as the firmwares
>> can be shared across OS's. Although this is how remoteproc subsys core
>> is currently working. It seems a generic DT firmware binding would actually
>> be most useful for the remoteproc subsystem.
>
> The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
> RProc driver is welcome to supply a different firmware name if it
> desires.  This is where I think a generic 'firmware' property would be
> of use.

Hmnnn... That default looks really inappropriate for the ST platforms!

Different generations of the hardware have co-processors with similar 
roles (and hence all with names like st231-gp0) but its very rare for 
the co-processors to have the same firmware binary.

Thus if the default were applied naively we will end up with a bizarre 
situation where the kernel is multi-platform but because the kernel does 
not select a unique name for the different firmwares, it becomes 
needlessly difficult for the userspace to support multiple platforms.

If there was a generic firmware property for this kind of situation it 
is fairly natural to note in the bindings docs the importance of 
uniqueness through the generations regarding of the choice of name.

There are other ways a driver can generate a unique name as the 
generations of chip develop (compatible strings actually very good for 
this) but I don't see it fitting quite so naturally into the binding 
docs where it can help future adventurers.


>> I guess I should also comment about this on the ST remoteproc thread.
>>
>> I'm curious though as to how the ST remoteproc driver then loads the firmware,
>> as that name doesn't look like a video or audio firmware filename that I've
>> seen shipped from ST. IIRC Usually it is named something like
>>
>> vid_firmware-stih407.elf or audio_firmware-bd-stih407.elf
>
> This driver came direct from ST.  I'm using their conventions as a
> default, although I would be happy to conduct some investigation into
> how this works for releases and suggest a better interface.  The only
> reason I mentioned it here was because this is how others are using
> other SS which are similar in some way.  Same for the Firmware SS I
> guess.
>
> In my unbiased PoV, I'd much prefer the name was derived from a
> predefined propertly.
>
>> IMO we should be treating the firmware as a blackbox, and for me that also
>> includes the filename it is shipped with. Linux drivers making up their own
>> firmware names based on the name of their subsystem doesn't seem like a
>> good way forward to me.
>
> I guess the driver author was just using the fall-back convention for
> testing.  I'm pretty sure RemoteProc isn't being shipped in products
> yet.

Both for fdma and co-processors, it is important that the kernel (or 
kernel informed by DT properties/compatible strings) is "assertive" 
enough to choose names that are sufficiently unique.

That is far more important than honoring vendor filenames.

Vendor filenames could easily be both too simple ("firmware.bin") or too 
detailed ("fdma-foo+bar-v1.8_20140811.fw"; the DT should not be encoding 
the version number of a firmware loaded from userspace since its not a 
fixed property of the platform).


Daniel.

Re: st_fdma: Firmware filename in DT?

[Rob Herring]

On Fri, Sep 4, 2015 at 9:27 AM, Warner Losh <imp-uzTCJ5RojNnQT0dZR+AlfA@public.gmane.org> wrote:
>
>> On Sep 4, 2015, at 4:21 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>
>>>>>> We could do it by parsing the node name e.g. fdma0-audio, or by adding
>>>>>> a "instance" DT property to the node?
>>>>>
>>>>> Generally we try to avoid caring about node names. Having some index
>>>>> or numbering also comes up which we also try to avoid. Generally, if
>>>>> you care about which instance you use for something, then there is
>>>>> some property you care about and should add.
>>>>
>>>> Right, the alternative is a property like the ones already used.
>>>> However, as these are becoming more prevalent I suggested
>>>> standardising the property to avoid all these vendor specific firmware
>>>> properties cluttering up the place.
>>>
>>> I agree a proliferation of vendor specific firmware properties isn't
>>> s good way forward.
>>>
>>>>  firmware = "firmwarename.fw";
>>>> OR
>>>>  firmware-name = "firmwarename.fw";
>>>>
>>>> ... seems appropriate.
>>>
>>> Either of those is fine with me.
>>
>> Just need a DT nod and I'll happily code it up.
>
> From a FreeBSD perspective, having a filename for the firmware to
> load for this node is fine. It doesn’t impose a substantial burden on
> the OS so long as the choices of where that file lives is up to the OS
> and not encoded in the property. A note saying ‘this firmware should
> be loaded’ seems a reasonable description of the hardware since
> the DT tells us many things about the device, and those things may
> well be dependent on which firmware is loaded. Having an explicit
> name is good since it helps insulate from DT and doesn’t force vendors
> to do silly renames. It also allows for multiple devices of the same
> type to have different firmware loaded.
>
> FreeBSD would generally put these things in /boot/firmware and
> it has a generalized mechanism to load the firmware at run time
> that’s based on this. While the path names are flexible, having
> the firmware live in a central place is useful from an automation
> point of view. Having just a name, and not a full path, enables
> this policy, while still allowing others to have other policies.
>
> Linux distributions would be free to do whatever they wanted
> and implement other policies than FreeBSD.
>
> So a property like this, with the semantics discussed, seems to
> meet the OS independent test.

Good. I'll await a binding to review...

I would suggest "firmware-names" to be consistent with other naming
convention and because their can be more than one (either at the same
time or as fallback). It also leaves "firmware" available if we want
to have phandle's to firmware embedded in the DT at some point later.

Rob

Re: st_fdma: Firmware filename in DT?

[Arnd Bergmann]

On Friday 04 September 2015, Warner Losh wrote:
> > On Sep 4, 2015, at 7:04 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> > On Friday 04 September 2015, Lee Jones wrote:
> >>>> If we flip it the other way round, some subsystems derive the firmware
> >>>> name from the 'node name'.  For instance, our zeroth General Purpose
> >>>> Co-Processor RemoteProc driver has a corresponding node called
> >>>> 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
> >>>> suffix and et voilà, we load file:
> >>>> 
> >>>>  lib/firmware/rproc-st231-gp0-fw
> >>> 
> >>> IMO deriving from the node name seems fragile. Also imposing a linux'ism
> >>> "rproc" prefix on the firmware name doesn't seem correct as the firmwares
> >>> can be shared across OS's. Although this is how remoteproc subsys core
> >>> is currently working. It seems a generic DT firmware binding would actually
> >>> be most useful for the remoteproc subsystem.
> >> 
> >> The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
> >> RProc driver is welcome to supply a different firmware name if it
> >> desires.  This is where I think a generic 'firmware' property would be> 
> Warner
> 
> 

> >> of use.
> > 
> > The firmware file name is agreed on between the device driver and the
> > file system, so encoding the linux driver name in it seems appropriate.
> 
> Encoding the driver name is not OS independent. It fosters the
> impression that DT isn’t OS independent, but rather some silly
> Linux toy and hurts wider adaptation.

I meant encoding the driver name in the path for the firmware file,
e.g.

	 $DRIVER/$COMPATIBLE.bin

where $DRIVER is the name of the driver, and $COMPATIBLE is the name
from the compatible string.

> > Generally speaking, I'd say a good policy would be to try basing
> > the firmware name on the "compatible" property strings. That property
> > already contains a hierarchical list of models, which makes it particularly
> > easy to have firmware files for specific models or those that are shared
> > across multiple variations if necessary. Just ask for the most specific
> > compatible string first and try the more specific compatible strings
> > (with an appropriate prefix and/or postfix added by the driver) until
> > a file is found.
> 
> I think this is a horrible policy. It is an ugly kludge that is fragile to change.
> While it sounds cool, I don’t think it is really a viable one. It requires
> different compatibility names for different bits of hardware that might otherwise
> be the same just to get different firmware. Sometimes this may be OK,
> but it does seem needlessly limiting for systems that may have firmware
> “images” that are loaded into one hardware block, but actually control
> other blocks indirectly (where the different compat strings would be).

The idea is that the compatible string here not only encodes the hardware
but also how it is getting used. This is the same as what we do with other
programmable hardware, such as PHY controllers that can be either PCIe
or USB3 depending on some setting. The driver would match on the more generic
property that identifies the hardware, while the firmware can match on the
more specific one, if any, or be provided by the user. As an example,
you can have

	compatible = "fwmaker,firmwaretype", "hardwaremaker,thisdevice-instance2",
		"hardwaremaker,thisdevice-version-3.1", "hardwaremaker,thisdevice";

So the driver matches on the one of the last two strings (as any other driver),
and the firmware can match on the same string, or the user can provide an image
in the file system based on the instance (if there are multiple ones), or the
boot loader can predefine which function should get loaded into this one, based
on how the hardware is physically wired up.

The file name for loading the firmware can still get constructed from additional
properties, e.g. the instance can be appended by the driver instead of coming
from compatible if that makes more sense.

	Arnd

Re: st_fdma: Firmware filename in DT?

[Arnd Bergmann]

On Friday 04 September 2015, Lee Jones wrote:
> On Fri, 04 Sep 2015, Arnd Bergmann wrote:
> 
> > On Friday 04 September 2015, Lee Jones wrote:
> > > > > If we flip it the other way round, some subsystems derive the firmware
> > > > > name from the 'node name'.  For instance, our zeroth General Purpose
> > > > > Co-Processor RemoteProc driver has a corresponding node called
> > > > > 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
> > > > > suffix and et voilà, we load file:
> > > > > 
> > > > >   lib/firmware/rproc-st231-gp0-fw
> > > > 
> > > > IMO deriving from the node name seems fragile. Also imposing a linux'ism
> > > > "rproc" prefix on the firmware name doesn't seem correct as the firmwares
> > > > can be shared across OS's. Although this is how remoteproc subsys core
> > > > is currently working. It seems a generic DT firmware binding would actually
> > > > be most useful for the remoteproc subsystem.
> > > 
> > > The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
> > > RProc driver is welcome to supply a different firmware name if it
> > > desires.  This is where I think a generic 'firmware' property would be
> > > of use.
> > 
> > The firmware file name is agreed on between the device driver and the
> > file system, so encoding the linux driver name in it seems appropriate.
> > 
> > Generally speaking, I'd say a good policy would be to try basing
> > the firmware name on the "compatible" property strings. That property
> > already contains a hierarchical list of models, which makes it particularly
> > easy to have firmware files for specific models or those that are shared
> > across multiple variations if necessary. Just ask for the most specific
> > compatible string first and try the more specific compatible strings
> > (with an appropriate prefix and/or postfix added by the driver) until
> > a file is found.
> 
> It depends what you mean by "basing the firmware name on the
> \"compatible\" property" here.  If you mean actually renaming the
> firmware binary file to match a driver's compatible string, that's
> absolutely out of the question.  Firmwares are not only OS agnostic,
> but are also independent of any H/W description language a particular
> OS or platform might be using.  Using DT'isums to rename these
> binaries is not logical.
> 

I was thinking of naming the firmware and the compatible string the
same, and often enough that makes sense when both names are picked
by the same person. However, you make a good point that there are cases
where the firmware file already has a name based on some other
decision process and there may be good reasons not to rename the
file. In that case a driver writer can do a lookup table from
one to the other.

The downside of a lookup table of course is that you have to modify
the driver for each new firmware, but then again a lot of drivers
already require that, and others can come up with a policy that lets
you do a programmatic mapping from one to the other by picking
clever compatible strings.

> However, if you mean simply match on compatible string and supply the
> name from within the driver, that's closer to the mark (as then we can
> at least keep it in-house [kernel]), but it's still not particularly
> practical for the aforementioned reasons mentioned by Peter earlier.
> 
> Why not just create a new 'firmware' property?  Simples! [0]
> 
> [0] http://www.oxforddictionaries.com/definition/english/simples

My main thought was that loading a different firmware practically
always means that the device behaves in a (sometimes more, sometimes
less) different way, and we want to reflect that with a different
compatible string. Having a 1:1 relation between the two strings
enforces this.

	Arnd

Re: st_fdma: Firmware filename in DT?

[Arnd Bergmann]

On Friday 04 September 2015, Lee Jones wrote:
> On Fri, 04 Sep 2015, Arnd Bergmann wrote:
> 
> > On Friday 04 September 2015, Lee Jones wrote:
> > > > > If we flip it the other way round, some subsystems derive the firmware
> > > > > name from the 'node name'.  For instance, our zeroth General Purpose
> > > > > Co-Processor RemoteProc driver has a corresponding node called
> > > > > 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
> > > > > suffix and et voilà, we load file:
> > > > > 
> > > > >   lib/firmware/rproc-st231-gp0-fw
> > > > 
> > > > IMO deriving from the node name seems fragile. Also imposing a linux'ism
> > > > "rproc" prefix on the firmware name doesn't seem correct as the firmwares
> > > > can be shared across OS's. Although this is how remoteproc subsys core
> > > > is currently working. It seems a generic DT firmware binding would actually
> > > > be most useful for the remoteproc subsystem.
> > > 
> > > The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
> > > RProc driver is welcome to supply a different firmware name if it
> > > desires.  This is where I think a generic 'firmware' property would be
> > > of use.
> > 
> > The firmware file name is agreed on between the device driver and the
> > file system, so encoding the linux driver name in it seems appropriate.
> > 
> > Generally speaking, I'd say a good policy would be to try basing
> > the firmware name on the "compatible" property strings. That property
> > already contains a hierarchical list of models, which makes it particularly
> > easy to have firmware files for specific models or those that are shared
> > across multiple variations if necessary. Just ask for the most specific
> > compatible string first and try the more specific compatible strings
> > (with an appropriate prefix and/or postfix added by the driver) until
> > a file is found.
> 
> It depends what you mean by "basing the firmware name on the
> \"compatible\" property" here.  If you mean actually renaming the
> firmware binary file to match a driver's compatible string, that's
> absolutely out of the question.  Firmwares are not only OS agnostic,
> but are also independent of any H/W description language a particular
> OS or platform might be using.  Using DT'isums to rename these
> binaries is not logical.
> 

I was thinking of naming the firmware and the compatible string the
same, and often enough that makes sense when both names are picked
by the same person. However, you make a good point that there are cases
where the firmware file already has a name based on some other
decision process and there may be good reasons not to rename the
file. In that case a driver writer can do a lookup table from
one to the other.

The downside of a lookup table of course is that you have to modify
the driver for each new firmware, but then again a lot of drivers
already require that, and others can come up with a policy that lets
you do a programmatic mapping from one to the other by picking
clever compatible strings.

> However, if you mean simply match on compatible string and supply the
> name from within the driver, that's closer to the mark (as then we can
> at least keep it in-house [kernel]), but it's still not particularly
> practical for the aforementioned reasons mentioned by Peter earlier.
> 
> Why not just create a new 'firmware' property?  Simples! [0]
> 
> [0] http://www.oxforddictionaries.com/definition/english/simples

My main thought was that loading a different firmware practically
always means that the device behaves in a (sometimes more, sometimes
less) different way, and we want to reflect that with a different
compatible string. Having a 1:1 relation between the two strings
enforces this.

	Arnd

Re: st_fdma: Firmware filename in DT?

[Arnd Bergmann]

On Friday 04 September 2015, Rob Herring wrote:
> On Fri, Sep 4, 2015 at 9:27 AM, Warner Losh <imp-uzTCJ5RojNnQT0dZR+AlfA@public.gmane.org> wrote:
> >> On Sep 4, 2015, at 4:21 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >>>>>> We could do it by parsing the node name e.g. fdma0-audio, or by adding
> > FreeBSD would generally put these things in /boot/firmware and
> > it has a generalized mechanism to load the firmware at run time
> > that’s based on this. While the path names are flexible, having
> > the firmware live in a central place is useful from an automation
> > point of view. Having just a name, and not a full path, enables
> > this policy, while still allowing others to have other policies.
> >
> > Linux distributions would be free to do whatever they wanted
> > and implement other policies than FreeBSD.
> >
> > So a property like this, with the semantics discussed, seems to
> > meet the OS independent test.
> 
> Good. I'll await a binding to review...
> 
> I would suggest "firmware-names" to be consistent with other naming
> convention and because their can be more than one (either at the same
> time or as fallback). It also leaves "firmware" available if we want
> to have phandle's to firmware embedded in the DT at some point later.

Having list of strings would certainly make this more flexible than
just a single string, for the same reason as taking the list of
compatible values as the base, so +1 for "firmware-names" over "firmware".

If we decide to create a proper binding for standardizing this, we
might also want to standardize inline firmware blobs. I've worked
with systems in the past that wanted to include a device firmware blob
in their system firmware, and that blob was not freely distributable
While nondistributable device firmware is not something we want to encourage,
people are going to do it anyway and standardizing the method may be better
than not.

See drivers/net/ethernet/toshiba/spider_net.c for a particular example
I was thinking of. In this case, the system firmware can provide a blob
to the kernel driver in a propery, but the driver will first look at
the local file system for an updated image.

	Arnd.

Re: st_fdma: Firmware filename in DT?

[Arnd Bergmann]

On Friday 04 September 2015, Rob Herring wrote:
> On Fri, Sep 4, 2015 at 9:27 AM, Warner Losh <imp-uzTCJ5RojNnQT0dZR+AlfA@public.gmane.org> wrote:
> >> On Sep 4, 2015, at 4:21 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >>>>>> We could do it by parsing the node name e.g. fdma0-audio, or by adding
> > FreeBSD would generally put these things in /boot/firmware and
> > it has a generalized mechanism to load the firmware at run time
> > that’s based on this. While the path names are flexible, having
> > the firmware live in a central place is useful from an automation
> > point of view. Having just a name, and not a full path, enables
> > this policy, while still allowing others to have other policies.
> >
> > Linux distributions would be free to do whatever they wanted
> > and implement other policies than FreeBSD.
> >
> > So a property like this, with the semantics discussed, seems to
> > meet the OS independent test.
> 
> Good. I'll await a binding to review...
> 
> I would suggest "firmware-names" to be consistent with other naming
> convention and because their can be more than one (either at the same
> time or as fallback). It also leaves "firmware" available if we want
> to have phandle's to firmware embedded in the DT at some point later.

Having list of strings would certainly make this more flexible than
just a single string, for the same reason as taking the list of
compatible values as the base, so +1 for "firmware-names" over "firmware".

If we decide to create a proper binding for standardizing this, we
might also want to standardize inline firmware blobs. I've worked
with systems in the past that wanted to include a device firmware blob
in their system firmware, and that blob was not freely distributable
While nondistributable device firmware is not something we want to encourage,
people are going to do it anyway and standardizing the method may be better
than not.

See drivers/net/ethernet/toshiba/spider_net.c for a particular example
I was thinking of. In this case, the system firmware can provide a blob
to the kernel driver in a propery, but the driver will first look at
the local file system for an updated image.

	Arnd.

Re: st_fdma: Firmware filename in DT?

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 5268 bytes --]

On Sat, Sep 5, 2015 at 2:58 AM, Arnd Bergmann <arnd@arndb.de> wrote:

> On Friday 04 September 2015, Warner Losh wrote:
> > > On Sep 4, 2015, at 7:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Friday 04 September 2015, Lee Jones wrote:
> > >>>> If we flip it the other way round, some subsystems derive the
> firmware
> > >>>> name from the 'node name'.  For instance, our zeroth General Purpose
> > >>>> Co-Processor RemoteProc driver has a corresponding node called
> > >>>> 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a
> '-fw'
> > >>>> suffix and et voilà, we load file:
> > >>>>
> > >>>>  lib/firmware/rproc-st231-gp0-fw
> > >>>
> > >>> IMO deriving from the node name seems fragile. Also imposing a
> linux'ism
> > >>> "rproc" prefix on the firmware name doesn't seem correct as the
> firmwares
> > >>> can be shared across OS's. Although this is how remoteproc subsys
> core
> > >>> is currently working. It seems a generic DT firmware binding would
> actually
> > >>> be most useful for the remoteproc subsystem.
> > >>
> > >> The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
> > >> RProc driver is welcome to supply a different firmware name if it
> > >> desires.  This is where I think a generic 'firmware' property would
> be>
> > Warner
> >
> >
>
> > >> of use.
> > >
> > > The firmware file name is agreed on between the device driver and the
> > > file system, so encoding the linux driver name in it seems appropriate.
> >
> > Encoding the driver name is not OS independent. It fosters the
> > impression that DT isn’t OS independent, but rather some silly
> > Linux toy and hurts wider adaptation.
>
> I meant encoding the driver name in the path for the firmware file,
> e.g.
>
>          $DRIVER/$COMPATIBLE.bin
>
> where $DRIVER is the name of the driver, and $COMPATIBLE is the name
> from the compatible string.
>
> > > Generally speaking, I'd say a good policy would be to try basing
> > > the firmware name on the "compatible" property strings. That property
> > > already contains a hierarchical list of models, which makes it
> particularly
> > > easy to have firmware files for specific models or those that are
> shared
> > > across multiple variations if necessary. Just ask for the most specific
> > > compatible string first and try the more specific compatible strings
> > > (with an appropriate prefix and/or postfix added by the driver) until
> > > a file is found.
> >
> > I think this is a horrible policy. It is an ugly kludge that is fragile
> to change.
> > While it sounds cool, I don’t think it is really a viable one. It
> requires
> > different compatibility names for different bits of hardware that might
> otherwise
> > be the same just to get different firmware. Sometimes this may be OK,
> > but it does seem needlessly limiting for systems that may have firmware
> > “images” that are loaded into one hardware block, but actually control
> > other blocks indirectly (where the different compat strings would be).
>
> The idea is that the compatible string here not only encodes the hardware
> but also how it is getting used. This is the same as what we do with other
> programmable hardware, such as PHY controllers that can be either PCIe
> or USB3 depending on some setting. The driver would match on the more
> generic
> property that identifies the hardware, while the firmware can match on the
> more specific one, if any, or be provided by the user. As an example,
> you can have
>
>         compatible = "fwmaker,firmwaretype",
> "hardwaremaker,thisdevice-instance2",
>                 "hardwaremaker,thisdevice-version-3.1",
> "hardwaremaker,thisdevice";
>
> So the driver matches on the one of the last two strings (as any other
> driver),
> and the firmware can match on the same string, or the user can provide an
> image
> in the file system based on the instance (if there are multiple ones), or
> the
> boot loader can predefine which function should get loaded into this one,
> based
> on how the hardware is physically wired up.
>
> The file name for loading the firmware can still get constructed from
> additional
> properties, e.g. the instance can be appended by the driver instead of
> coming
> from compatible if that makes more sense
>

I understood what you were proposing. It wasn't a lack of understanding
that lead me to the opinion that it was a horrible policy. Despite your
lengthy explanation, you did nothing to address the very simple and common
use case of having identical hardware that none-the-less needs different
firmware which was my basis of the horrible brand I put on this policy.

Also, firmware comes from vendors generally named something other
than FDT compat strings. Renaming firmware generally is also a
horrible policy that most shops frown on. It hampers traceability from
a deployed system back to the sources, for one and introduces one
more place for an 'oops' rename to go undetected until the firmware
is deployed.

So there needs to be a standardized way to augment local policy
to say 'the firmware you need for this node so that the system behaves
as expect in the DT is Y.'

Warner

[-- Attachment #2: Type: text/html, Size: 6463 bytes --]

Re: st_fdma: Firmware filename in DT?

[Daniel Thompson]

On 05/09/15 10:25, Arnd Bergmann wrote:
> On Friday 04 September 2015, Rob Herring wrote:
>> On Fri, Sep 4, 2015 at 9:27 AM, Warner Losh <imp-uzTCJ5RojNnQT0dZR+AlfA@public.gmane.org> wrote:
>>>> On Sep 4, 2015, at 4:21 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>>>>>> We could do it by parsing the node name e.g. fdma0-audio, or by adding
>>> FreeBSD would generally put these things in /boot/firmware and
>>> it has a generalized mechanism to load the firmware at run time
>>> that’s based on this. While the path names are flexible, having
>>> the firmware live in a central place is useful from an automation
>>> point of view. Having just a name, and not a full path, enables
>>> this policy, while still allowing others to have other policies.
>>>
>>> Linux distributions would be free to do whatever they wanted
>>> and implement other policies than FreeBSD.
>>>
>>> So a property like this, with the semantics discussed, seems to
>>> meet the OS independent test.
>>
>> Good. I'll await a binding to review...
>>
>> I would suggest "firmware-names" to be consistent with other naming
>> convention and because their can be more than one (either at the same
>> time or as fallback). It also leaves "firmware" available if we want
>> to have phandle's to firmware embedded in the DT at some point later.
>
> Having list of strings would certainly make this more flexible than
> just a single string, for the same reason as taking the list of
> compatible values as the base, so +1 for "firmware-names" over "firmware".

I was recently thinking about how DT filenames would interact with 
incompatible ABI changes to the firmware's register and/or wire protocol.

Just for example we start with f/ware ABI A and we create a mainline 
kernel X that supported it.

Vendor now releases a new firmware with ABI B and we update the mainline 
driver to support ABI A (to avoid breaking old userspaces) and B, 
eventually releasing kernel Y.

The question now is how kernels X and Y should use the DT to generate 
the filename. It is very desirable that kernels X and Y use *different* 
filenames because otherwise a single userspace could not support the new 
feature *and* boot with both X and Y.

Having lists of firmwares can certainly help solve this (providing the 
list can be used to allow a driver to select for an ABI is supports). 
However I afraid I find this example argues against having filenames in 
DT at all because it seems odd to me that, for kernel and userspace to 
adopt ABI B that must wait until the DT is updated to include support 
for ABI B. The hardware didn't change...


Daniel.

Re: st_fdma: Firmware filename in DT?

[Daniel Thompson]

On 05/09/15 10:25, Arnd Bergmann wrote:
> On Friday 04 September 2015, Rob Herring wrote:
>> On Fri, Sep 4, 2015 at 9:27 AM, Warner Losh <imp-uzTCJ5RojNnQT0dZR+AlfA@public.gmane.org> wrote:
>>>> On Sep 4, 2015, at 4:21 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>>>>>> We could do it by parsing the node name e.g. fdma0-audio, or by adding
>>> FreeBSD would generally put these things in /boot/firmware and
>>> it has a generalized mechanism to load the firmware at run time
>>> that’s based on this. While the path names are flexible, having
>>> the firmware live in a central place is useful from an automation
>>> point of view. Having just a name, and not a full path, enables
>>> this policy, while still allowing others to have other policies.
>>>
>>> Linux distributions would be free to do whatever they wanted
>>> and implement other policies than FreeBSD.
>>>
>>> So a property like this, with the semantics discussed, seems to
>>> meet the OS independent test.
>>
>> Good. I'll await a binding to review...
>>
>> I would suggest "firmware-names" to be consistent with other naming
>> convention and because their can be more than one (either at the same
>> time or as fallback). It also leaves "firmware" available if we want
>> to have phandle's to firmware embedded in the DT at some point later.
>
> Having list of strings would certainly make this more flexible than
> just a single string, for the same reason as taking the list of
> compatible values as the base, so +1 for "firmware-names" over "firmware".

I was recently thinking about how DT filenames would interact with 
incompatible ABI changes to the firmware's register and/or wire protocol.

Just for example we start with f/ware ABI A and we create a mainline 
kernel X that supported it.

Vendor now releases a new firmware with ABI B and we update the mainline 
driver to support ABI A (to avoid breaking old userspaces) and B, 
eventually releasing kernel Y.

The question now is how kernels X and Y should use the DT to generate 
the filename. It is very desirable that kernels X and Y use *different* 
filenames because otherwise a single userspace could not support the new 
feature *and* boot with both X and Y.

Having lists of firmwares can certainly help solve this (providing the 
list can be used to allow a driver to select for an ABI is supports). 
However I afraid I find this example argues against having filenames in 
DT at all because it seems odd to me that, for kernel and userspace to 
adopt ABI B that must wait until the DT is updated to include support 
for ABI B. The hardware didn't change...


Daniel.

Re: st_fdma: Firmware filename in DT?

[Arnd Bergmann]

On Monday 07 September 2015 11:30:11 Daniel Thompson wrote:
> >>
> >> I would suggest "firmware-names" to be consistent with other naming
> >> convention and because their can be more than one (either at the same
> >> time or as fallback). It also leaves "firmware" available if we want
> >> to have phandle's to firmware embedded in the DT at some point later.
> >
> > Having list of strings would certainly make this more flexible than
> > just a single string, for the same reason as taking the list of
> > compatible values as the base, so +1 for "firmware-names" over "firmware".
> 
> I was recently thinking about how DT filenames would interact with 
> incompatible ABI changes to the firmware's register and/or wire protocol.
> 
> Just for example we start with f/ware ABI A and we create a mainline 
> kernel X that supported it.
> 
> Vendor now releases a new firmware with ABI B and we update the mainline 
> driver to support ABI A (to avoid breaking old userspaces) and B, 
> eventually releasing kernel Y.
> 
> The question now is how kernels X and Y should use the DT to generate 
> the filename. It is very desirable that kernels X and Y use *different* 
> filenames because otherwise a single userspace could not support the new 
> feature *and* boot with both X and Y.

Generally speaking, the kernel should shield user space from ABI
differences in the firmware and still provide the same user space ABI
(possibly emulated, when the newer firmware removes features of the
old one). Can you think of a case where a device firmware directly
defines the user ABI without having kernel visible changes?

> Having lists of firmwares can certainly help solve this (providing the 
> list can be used to allow a driver to select for an ABI is supports). 
> However I afraid I find this example argues against having filenames in 
> DT at all because it seems odd to me that, for kernel and userspace to 
> adopt ABI B that must wait until the DT is updated to include support 
> for ABI B. The hardware didn't change...

This is not what I was thinking of for a list of file names: I would not
want a case where the kernel might pick between multiple incompatible
files without knowing what they are, especially if the differences
are ABI relevant.

However, being very specific would let users intentionally install a
firmware that is only used on a particular instance of a device that
they want to behave differently, while the generic firmware would come
preinstalled with the normal linux-firmware package.

	Arnd

Re: st_fdma: Firmware filename in DT?

[Arnd Bergmann]

On Saturday 05 September 2015 15:06:35 Warner Losh wrote:
> On Sat, Sep 5, 2015 at 2:58 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> > On Friday 04 September 2015, Warner Losh wrote:
> 
> I understood what you were proposing. It wasn't a lack of understanding
> that lead me to the opinion that it was a horrible policy. Despite your
> lengthy explanation, you did nothing to address the very simple and common
> use case of having identical hardware that none-the-less needs different
> firmware which was my basis of the horrible brand I put on this policy.

You must be thinking of different examples of firmware files than I
am. We have tons of drivers that need firmware, and the reasons for
picking a particular file over another alternative tend to be slightly
different (most of the time, you just want the latest version).

Can you name a specific example you are thinking of where you want
different firmware to be loaded on systems with identical hardware?

> Also, firmware comes from vendors generally named something other
> than FDT compat strings. Renaming firmware generally is also a
> horrible policy that most shops frown on. It hampers traceability from
> a deployed system back to the sources, for one and introduces one
> more place for an 'oops' rename to go undetected until the firmware
> is deployed.

Most firmware that I can think of does not even come as a file in the
format that Linux wants, often there is some vendor source code
with firmware in a header file, or you dissect a windows binary
driver to pull out the right bits.

The file name gets fixed at the point at which the binary is included
in the linux-firmware git tree.

> So there needs to be a standardized way to augment local policy
> to say 'the firmware you need for this node so that the system behaves
> as expect in the DT is Y.'

Why is that even something that is known by the boot loader? If the boot
loader just knows what hardware you have, but a particular instance
requires some other device firmware, that sounds like the system
administrator would just put the special firmware file in a known
location on the local file system (preferably not the same file name
as the generic one).

	Arnd

Re: st_fdma: Firmware filename in DT?

[Daniel Thompson]

On 07/09/15 13:33, Arnd Bergmann wrote:
> On Monday 07 September 2015 11:30:11 Daniel Thompson wrote:
>>>>
>>>> I would suggest "firmware-names" to be consistent with other naming
>>>> convention and because their can be more than one (either at the same
>>>> time or as fallback). It also leaves "firmware" available if we want
>>>> to have phandle's to firmware embedded in the DT at some point later.
>>>
>>> Having list of strings would certainly make this more flexible than
>>> just a single string, for the same reason as taking the list of
>>> compatible values as the base, so +1 for "firmware-names" over "firmware".
>>
>> I was recently thinking about how DT filenames would interact with
>> incompatible ABI changes to the firmware's register and/or wire protocol.
>>
>> Just for example we start with f/ware ABI A and we create a mainline
>> kernel X that supported it.
>>
>> Vendor now releases a new firmware with ABI B and we update the mainline
>> driver to support ABI A (to avoid breaking old userspaces) and B,
>> eventually releasing kernel Y.
>>
>> The question now is how kernels X and Y should use the DT to generate
>> the filename. It is very desirable that kernels X and Y use *different*
>> filenames because otherwise a single userspace could not support the new
>> feature *and* boot with both X and Y.
>
> Generally speaking, the kernel should shield user space from ABI
> differences in the firmware and still provide the same user space ABI
> (possibly emulated, when the newer firmware removes features of the
> old one). Can you think of a case where a device firmware directly
> defines the user ABI without having kernel visible changes?

I don't mean that the firmware ABI is exposed to userspace.

I mean if you use the same filename for both ABIs (in /lib/firmware) 
then kernel X with lose functionality if it is booted with a userspace 
that has updated to the new firmware (maybe even crash if I can't detect 
at runtime the version of the firmware is has loaded). That sort of 
thing is the pain for distros which support booting with multiple 
kernels (apt-get/dnf simply won't know when it is safe to update the 
firmware binary).


>> Having lists of firmwares can certainly help solve this (providing the
>> list can be used to allow a driver to select for an ABI is supports).
>> However I afraid I find this example argues against having filenames in
>> DT at all because it seems odd to me that, for kernel and userspace to
>> adopt ABI B that must wait until the DT is updated to include support
>> for ABI B. The hardware didn't change...
>
> This is not what I was thinking of for a list of file names: I would not
> want a case where the kernel might pick between multiple incompatible
> files without knowing what they are, especially if the differences
> are ABI relevant.

Nor me.

Personally I prefer an "assertive" driver that imposes a filename for 
the firmwares it needs. Thus if there is an ABI change the driver writer 
can load a completely different firmware file without needing a DT 
update. That said I'm not especially invested either way.

However whatever solution is reached I would quite like to be able to 
boot kernels from either side of an ABI break from a single userspace 
(and get the new features when the kernel supports them).


> However, being very specific would let users intentionally install a
> firmware that is only used on a particular instance of a device that
> they want to behave differently, while the generic firmware would come
> preinstalled with the normal linux-firmware package.

Not sure I follow this. You mean embedding filenames in DT makes it easy 
to something like a vendor/reverse-engineering mode.


Daniel.

Re: st_fdma: Firmware filename in DT?

[Arnd Bergmann]

On Monday 07 September 2015 15:36:27 Daniel Thompson wrote:
> On 07/09/15 13:33, Arnd Bergmann wrote:
> > On Monday 07 September 2015 11:30:11 Daniel Thompson wrote:
>
> > Generally speaking, the kernel should shield user space from ABI
> > differences in the firmware and still provide the same user space ABI
> > (possibly emulated, when the newer firmware removes features of the
> > old one). Can you think of a case where a device firmware directly
> > defines the user ABI without having kernel visible changes?
> 
> I don't mean that the firmware ABI is exposed to userspace.
> 
> I mean if you use the same filename for both ABIs (in /lib/firmware) 
> then kernel X with lose functionality if it is booted with a userspace 
> that has updated to the new firmware (maybe even crash if I can't detect 
> at runtime the version of the firmware is has loaded). That sort of 
> thing is the pain for distros which support booting with multiple 
> kernels (apt-get/dnf simply won't know when it is safe to update the 
> firmware binary).

I see. This is the case I was interested in with tying the compatible
string to the firmware blob: the compatible string tells us which
device we are dealing with, and if that is implemented in firmware,
then loading the firmware based on the same string will make everything
work, while having a separate DT property can cause trouble as the
driver now has to check two properties to find out what the ABI is.

Another case is where a driver knows of two firmware ABIs for the same
hardware and the boot loader does not care which one is used. In this
case, we also don't want to encode the firmware name in DT, but the driver
has to know anyway, and I can see two ways of doing this:

a) have version information inside of the blob. This works if the driver
   developer also controls the image format (as is often the case).
   Another downside is that it fails if the firmware image uses an ABI
   that the (older) kernel does not understand.

b) append the ABI version to the file name and try loading the firmware
   for the latest interface first, then go back to increasingly older
   file names that the driver still understands. This is already common
   practice in some drivers, regardless of matching the file name to
   DT.

> >> Having lists of firmwares can certainly help solve this (providing the
> >> list can be used to allow a driver to select for an ABI is supports).
> >> However I afraid I find this example argues against having filenames in
> >> DT at all because it seems odd to me that, for kernel and userspace to
> >> adopt ABI B that must wait until the DT is updated to include support
> >> for ABI B. The hardware didn't change...
> >
> > This is not what I was thinking of for a list of file names: I would not
> > want a case where the kernel might pick between multiple incompatible
> > files without knowing what they are, especially if the differences
> > are ABI relevant.
> 
> Nor me.
> 
> Personally I prefer an "assertive" driver that imposes a filename for 
> the firmwares it needs. Thus if there is an ABI change the driver writer 
> can load a completely different firmware file without needing a DT 
> update. That said I'm not especially invested either way.
> 
> However whatever solution is reached I would quite like to be able to 
> boot kernels from either side of an ABI break from a single userspace 
> (and get the new features when the kernel supports them).
> 
> 
> > However, being very specific would let users intentionally install a
> > firmware that is only used on a particular instance of a device that
> > they want to behave differently, while the generic firmware would come
> > preinstalled with the normal linux-firmware package.
> 
> Not sure I follow this. You mean embedding filenames in DT makes it easy 
> to something like a vendor/reverse-engineering mode.

The example I was thinking of is drivers/net/wireless/ath/ath10k/, which
first attempts to load the firmware image based on the specific PCI
vendor/device/subvendor/subdevice ID, then the vendor/device tuple and
finally falls back to just the driver name (or something similar to that,
I haven't looked up the exact method).

	Arnd

Re: st_fdma: Firmware filename in DT?

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1291 bytes --]

On Thu, Sep 03, 2015 at 04:45:35PM -0500, Rob Herring wrote:
> +devicetree-spec as a good question to separate from the fire hose.
> 
> On Thu, Sep 3, 2015 at 9:49 AM, Peter Griffin <peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > Hi Rob, Pawel, Mark, Ian and Kumar,
> >
> > Quick question regarding this series here
> > https://lkml.org/lkml/2015/7/8/832. and the proposed
> > st,fw-name binding.
> 
> Thanks for looking at the bigger picture.
> 
> > What are the rules with putting firmware names into DT?
> 
> Whatever you can sneak in without DT maintainers noticing...
> 
> > Is it allowed?
> 
> They are already there as you have found, so yes. But should they be
> allowed? Possibly. I'm not saying no, but do have some concerns.

I think this is a genuine edge case.  A firmware name isn't strictly
speaking hardware description, but if the names exist in some "well
known" OS independent namespace, then it's a reasonable thing to be
specified in the device tree.

That said, I have some concerns on points, see later replies to the thread.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: st_fdma: Firmware filename in DT?

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5744 bytes --]

On Sat, Sep 05, 2015 at 11:17:59AM +0200, Arnd Bergman wrote:
> On Friday 04 September 2015, Lee Jones wrote:
> > On Fri, 04 Sep 2015, Arnd Bergmann wrote:
> > 
> > > On Friday 04 September 2015, Lee Jones wrote:
> > > > > > If we flip it the other way round, some subsystems derive the firmware
> > > > > > name from the 'node name'.  For instance, our zeroth General Purpose
> > > > > > Co-Processor RemoteProc driver has a corresponding node called
> > > > > > 'st231-gp0@40000000'.  RemoteProc adds an 'rproc-' prefix and a '-fw'
> > > > > > suffix and et voilà, we load file:
> > > > > > 
> > > > > >   lib/firmware/rproc-st231-gp0-fw
> > > > > 
> > > > > IMO deriving from the node name seems fragile. Also imposing a linux'ism
> > > > > "rproc" prefix on the firmware name doesn't seem correct as the firmwares
> > > > > can be shared across OS's. Although this is how remoteproc subsys core
> > > > > is currently working. It seems a generic DT firmware binding would actually
> > > > > be most useful for the remoteproc subsystem.
> > > > 
> > > > The "rproc-%s-fw", where %s == driver name, is only a fall-back.  The
> > > > RProc driver is welcome to supply a different firmware name if it
> > > > desires.  This is where I think a generic 'firmware' property would be
> > > > of use.
> > > 
> > > The firmware file name is agreed on between the device driver and the
> > > file system, so encoding the linux driver name in it seems appropriate.
> > > 
> > > Generally speaking, I'd say a good policy would be to try basing
> > > the firmware name on the "compatible" property strings. That property
> > > already contains a hierarchical list of models, which makes it particularly
> > > easy to have firmware files for specific models or those that are shared
> > > across multiple variations if necessary. Just ask for the most specific
> > > compatible string first and try the more specific compatible strings
> > > (with an appropriate prefix and/or postfix added by the driver) until
> > > a file is found.
> > 
> > It depends what you mean by "basing the firmware name on the
> > \"compatible\" property" here.  If you mean actually renaming the
> > firmware binary file to match a driver's compatible string, that's
> > absolutely out of the question.  Firmwares are not only OS agnostic,
> > but are also independent of any H/W description language a particular
> > OS or platform might be using.  Using DT'isums to rename these
> > binaries is not logical.
> 
> I was thinking of naming the firmware and the compatible string the
> same, and often enough that makes sense when both names are picked
> by the same person. However, you make a good point that there are cases
> where the firmware file already has a name based on some other
> decision process and there may be good reasons not to rename the
> file. In that case a driver writer can do a lookup table from
> one to the other.

> The downside of a lookup table of course is that you have to modify
> the driver for each new firmware, but then again a lot of drivers
> already require that, and others can come up with a policy that lets
> you do a programmatic mapping from one to the other by picking
> clever compatible strings.

You can avoid an in-driver lookup table, though, if you use the
compatible string as an "alias" name to the canonical firmware name -
implemented with a symlink in userspace.

> > However, if you mean simply match on compatible string and supply the
> > name from within the driver, that's closer to the mark (as then we can
> > at least keep it in-house [kernel]), but it's still not particularly
> > practical for the aforementioned reasons mentioned by Peter earlier.
> > 
> > Why not just create a new 'firmware' property?  Simples! [0]
> > 
> > [0] http://www.oxforddictionaries.com/definition/english/simples
> 
> My main thought was that loading a different firmware practically
> always means that the device behaves in a (sometimes more, sometimes
> less) different way, and we want to reflect that with a different
> compatible string. Having a 1:1 relation between the two strings
> enforces this.

I think this is an excellent point.  Enforcing that the compatible and
firmware travel together is actually a really good idea.  As I see it
there are two cases where separating them seems like a good idea, but
actually isn't:

1) The hardware is identical, and different firmware is used to apply
   it in different ways.

   In this case the firmware change is a usage question rather than
   hardware description and doesn't belong in the device tree.

2) There are several low-level variants on the hardware, which require
   different firmware images in order to present the same programming
   interface to the OS.

   In this case there's a very high chance that some firmware revision
   will have bugs meaning it's not *quite* compatible with the common
   programming interface.  So, it's correct to present a different
   compatible property which a driver can use if it needs to work
   around those bugs.

So, I think selecting the firmware based on the compatible property is
the correct choice for the DT.

Whether the OS uses an in-driver lookup table to go from compatible
property to firmware name, or the firmware images are renamed to match
the compatible property, or symlinks are used to put the lookup table
into the filesystem is an OS design question that shouldn't affect DT
binding design.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: st_fdma: Firmware filename in DT?

[Peter Griffin]

Hi,

On Mon, 07 Sep 2015, Daniel Thompson wrote:

> On 07/09/15 13:33, Arnd Bergmann wrote:
> >On Monday 07 September 2015 11:30:11 Daniel Thompson wrote:
> >>>>
> >>>>I would suggest "firmware-names" to be consistent with other naming
> >>>>convention and because their can be more than one (either at the same
> >>>>time or as fallback). It also leaves "firmware" available if we want
> >>>>to have phandle's to firmware embedded in the DT at some point later.
> >>>
> >>>Having list of strings would certainly make this more flexible than
> >>>just a single string, for the same reason as taking the list of
> >>>compatible values as the base, so +1 for "firmware-names" over "firmware".
> >>
> >>I was recently thinking about how DT filenames would interact with
> >>incompatible ABI changes to the firmware's register and/or wire protocol.
> >>
> >>Just for example we start with f/ware ABI A and we create a mainline
> >>kernel X that supported it.
> >>
> >>Vendor now releases a new firmware with ABI B and we update the mainline
> >>driver to support ABI A (to avoid breaking old userspaces) and B,
> >>eventually releasing kernel Y.
> >>
> >>The question now is how kernels X and Y should use the DT to generate
> >>the filename. It is very desirable that kernels X and Y use *different*
> >>filenames because otherwise a single userspace could not support the new
> >>feature *and* boot with both X and Y.
> >
> >Generally speaking, the kernel should shield user space from ABI
> >differences in the firmware and still provide the same user space ABI
> >(possibly emulated, when the newer firmware removes features of the
> >old one). Can you think of a case where a device firmware directly
> >defines the user ABI without having kernel visible changes?
> 
> I don't mean that the firmware ABI is exposed to userspace.
> 
> I mean if you use the same filename for both ABIs (in /lib/firmware)
> then kernel X with lose functionality if it is booted with a
> userspace that has updated to the new firmware (maybe even crash if
> I can't detect at runtime the version of the firmware is has
> loaded). That sort of thing is the pain for distros which support
> booting with multiple kernels (apt-get/dnf simply won't know when it
> is safe to update the firmware binary).
> 
> 
> >>Having lists of firmwares can certainly help solve this (providing the
> >>list can be used to allow a driver to select for an ABI is supports).
> >>However I afraid I find this example argues against having filenames in
> >>DT at all because it seems odd to me that, for kernel and userspace to
> >>adopt ABI B that must wait until the DT is updated to include support
> >>for ABI B. The hardware didn't change...

This is a really good point and one I'd not considered.

> >
> >This is not what I was thinking of for a list of file names: I would not
> >want a case where the kernel might pick between multiple incompatible
> >files without knowing what they are, especially if the differences
> >are ABI relevant.
> 
> Nor me.
> 
> Personally I prefer an "assertive" driver that imposes a filename
> for the firmwares it needs. Thus if there is an ABI change the
> driver writer can load a completely different firmware file without
> needing a DT update. That said I'm not especially invested either
> way.
> 
> However whatever solution is reached I would quite like to be able
> to boot kernels from either side of an ABI break from a single
> userspace (and get the new features when the kernel supports them).

So to summarise, I think given Daniels good points about ABI
compatability, and also David and Arnd PoV, it would be best to leave
the generation of the firmware filename to the individual driver and
not put the filename in DT.

The driver can then generate a unique filename based off of the
compatible string.

I will send an updated version of the fdma patches which generate
the filename in this way.

Many thanks to you all for your valuable input!

regards,

Peter.

Re: st_fdma: Firmware filename in DT?

[Lee Jones]

On Thu, 10 Sep 2015, Peter Griffin wrote:
> On Mon, 07 Sep 2015, Daniel Thompson wrote:
> > On 07/09/15 13:33, Arnd Bergmann wrote:
> > >On Monday 07 September 2015 11:30:11 Daniel Thompson wrote:
> > >>>>
> > >>>>I would suggest "firmware-names" to be consistent with other naming
> > >>>>convention and because their can be more than one (either at the same
> > >>>>time or as fallback). It also leaves "firmware" available if we want
> > >>>>to have phandle's to firmware embedded in the DT at some point later.
> > >>>
> > >>>Having list of strings would certainly make this more flexible than
> > >>>just a single string, for the same reason as taking the list of
> > >>>compatible values as the base, so +1 for "firmware-names" over "firmware".
> > >>
> > >>I was recently thinking about how DT filenames would interact with
> > >>incompatible ABI changes to the firmware's register and/or wire protocol.
> > >>
> > >>Just for example we start with f/ware ABI A and we create a mainline
> > >>kernel X that supported it.
> > >>
> > >>Vendor now releases a new firmware with ABI B and we update the mainline
> > >>driver to support ABI A (to avoid breaking old userspaces) and B,
> > >>eventually releasing kernel Y.
> > >>
> > >>The question now is how kernels X and Y should use the DT to generate
> > >>the filename. It is very desirable that kernels X and Y use *different*
> > >>filenames because otherwise a single userspace could not support the new
> > >>feature *and* boot with both X and Y.
> > >
> > >Generally speaking, the kernel should shield user space from ABI
> > >differences in the firmware and still provide the same user space ABI
> > >(possibly emulated, when the newer firmware removes features of the
> > >old one). Can you think of a case where a device firmware directly
> > >defines the user ABI without having kernel visible changes?
> > 
> > I don't mean that the firmware ABI is exposed to userspace.
> > 
> > I mean if you use the same filename for both ABIs (in /lib/firmware)
> > then kernel X with lose functionality if it is booted with a
> > userspace that has updated to the new firmware (maybe even crash if
> > I can't detect at runtime the version of the firmware is has
> > loaded). That sort of thing is the pain for distros which support
> > booting with multiple kernels (apt-get/dnf simply won't know when it
> > is safe to update the firmware binary).
> > 
> > 
> > >>Having lists of firmwares can certainly help solve this (providing the
> > >>list can be used to allow a driver to select for an ABI is supports).
> > >>However I afraid I find this example argues against having filenames in
> > >>DT at all because it seems odd to me that, for kernel and userspace to
> > >>adopt ABI B that must wait until the DT is updated to include support
> > >>for ABI B. The hardware didn't change...
> 
> This is a really good point and one I'd not considered.
> 
> > >
> > >This is not what I was thinking of for a list of file names: I would not
> > >want a case where the kernel might pick between multiple incompatible
> > >files without knowing what they are, especially if the differences
> > >are ABI relevant.
> > 
> > Nor me.
> > 
> > Personally I prefer an "assertive" driver that imposes a filename
> > for the firmwares it needs. Thus if there is an ABI change the
> > driver writer can load a completely different firmware file without
> > needing a DT update. That said I'm not especially invested either
> > way.
> > 
> > However whatever solution is reached I would quite like to be able
> > to boot kernels from either side of an ABI break from a single
> > userspace (and get the new features when the kernel supports them).
> 
> So to summarise, I think given Daniels good points about ABI
> compatability, and also David and Arnd PoV, it would be best to leave
> the generation of the firmware filename to the individual driver and
> not put the filename in DT.

<hijacking>

That doesn't work for middle-layer drivers such as Remoteproc, where
it doesn't have its own associated firmwares.  Remoteproc's job is
simply to load the firmware.  It doesn't care which version of the ABI
that particular binary uses, and has no reason to.  Ideally, I guess
the Remoteproc client should be providing the firmware name, but why
should the client care who or what was used to load the firmware?

</hijacking>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: st_fdma: Firmware filename in DT?

[Lee Jones]

On Thu, 10 Sep 2015, Peter Griffin wrote:
> On Mon, 07 Sep 2015, Daniel Thompson wrote:
> > On 07/09/15 13:33, Arnd Bergmann wrote:
> > >On Monday 07 September 2015 11:30:11 Daniel Thompson wrote:
> > >>>>
> > >>>>I would suggest "firmware-names" to be consistent with other naming
> > >>>>convention and because their can be more than one (either at the same
> > >>>>time or as fallback). It also leaves "firmware" available if we want
> > >>>>to have phandle's to firmware embedded in the DT at some point later.
> > >>>
> > >>>Having list of strings would certainly make this more flexible than
> > >>>just a single string, for the same reason as taking the list of
> > >>>compatible values as the base, so +1 for "firmware-names" over "firmware".
> > >>
> > >>I was recently thinking about how DT filenames would interact with
> > >>incompatible ABI changes to the firmware's register and/or wire protocol.
> > >>
> > >>Just for example we start with f/ware ABI A and we create a mainline
> > >>kernel X that supported it.
> > >>
> > >>Vendor now releases a new firmware with ABI B and we update the mainline
> > >>driver to support ABI A (to avoid breaking old userspaces) and B,
> > >>eventually releasing kernel Y.
> > >>
> > >>The question now is how kernels X and Y should use the DT to generate
> > >>the filename. It is very desirable that kernels X and Y use *different*
> > >>filenames because otherwise a single userspace could not support the new
> > >>feature *and* boot with both X and Y.
> > >
> > >Generally speaking, the kernel should shield user space from ABI
> > >differences in the firmware and still provide the same user space ABI
> > >(possibly emulated, when the newer firmware removes features of the
> > >old one). Can you think of a case where a device firmware directly
> > >defines the user ABI without having kernel visible changes?
> > 
> > I don't mean that the firmware ABI is exposed to userspace.
> > 
> > I mean if you use the same filename for both ABIs (in /lib/firmware)
> > then kernel X with lose functionality if it is booted with a
> > userspace that has updated to the new firmware (maybe even crash if
> > I can't detect at runtime the version of the firmware is has
> > loaded). That sort of thing is the pain for distros which support
> > booting with multiple kernels (apt-get/dnf simply won't know when it
> > is safe to update the firmware binary).
> > 
> > 
> > >>Having lists of firmwares can certainly help solve this (providing the
> > >>list can be used to allow a driver to select for an ABI is supports).
> > >>However I afraid I find this example argues against having filenames in
> > >>DT at all because it seems odd to me that, for kernel and userspace to
> > >>adopt ABI B that must wait until the DT is updated to include support
> > >>for ABI B. The hardware didn't change...
> 
> This is a really good point and one I'd not considered.
> 
> > >
> > >This is not what I was thinking of for a list of file names: I would not
> > >want a case where the kernel might pick between multiple incompatible
> > >files without knowing what they are, especially if the differences
> > >are ABI relevant.
> > 
> > Nor me.
> > 
> > Personally I prefer an "assertive" driver that imposes a filename
> > for the firmwares it needs. Thus if there is an ABI change the
> > driver writer can load a completely different firmware file without
> > needing a DT update. That said I'm not especially invested either
> > way.
> > 
> > However whatever solution is reached I would quite like to be able
> > to boot kernels from either side of an ABI break from a single
> > userspace (and get the new features when the kernel supports them).
> 
> So to summarise, I think given Daniels good points about ABI
> compatability, and also David and Arnd PoV, it would be best to leave
> the generation of the firmware filename to the individual driver and
> not put the filename in DT.

<hijacking>

That doesn't work for middle-layer drivers such as Remoteproc, where
it doesn't have its own associated firmwares.  Remoteproc's job is
simply to load the firmware.  It doesn't care which version of the ABI
that particular binary uses, and has no reason to.  Ideally, I guess
the Remoteproc client should be providing the firmware name, but why
should the client care who or what was used to load the firmware?

</hijacking>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: st_fdma: Firmware filename in DT?

[Arnd Bergmann]

On Friday 11 September 2015 10:17:00 Lee Jones wrote:
> <hijacking>
> 
> That doesn't work for middle-layer drivers such as Remoteproc, where
> it doesn't have its own associated firmwares.  Remoteproc's job is
> simply to load the firmware.  It doesn't care which version of the ABI
> that particular binary uses, and has no reason to.  Ideally, I guess
> the Remoteproc client should be providing the firmware name, but why
> should the client care who or what was used to load the firmware?
> 
> </hijacking>
> 

Does remoteproc use request_firmware() then? If not, it's irrelevant
to this discussion.

	Arnd

Re: st_fdma: Firmware filename in DT?

[Lee Jones]

On Fri, 11 Sep 2015, Arnd Bergmann wrote:

> On Friday 11 September 2015 10:17:00 Lee Jones wrote:
> > <hijacking>
> > 
> > That doesn't work for middle-layer drivers such as Remoteproc, where
> > it doesn't have its own associated firmwares.  Remoteproc's job is
> > simply to load the firmware.  It doesn't care which version of the ABI
> > that particular binary uses, and has no reason to.  Ideally, I guess
> > the Remoteproc client should be providing the firmware name, but why
> > should the client care who or what was used to load the firmware?
> > 
> > </hijacking>
> > 
> 
> Does remoteproc use request_firmware() then?

Yes ...

> If not, it's irrelevant to this discussion.

... but even if it didn't, it would still be relevant, as it's a
"should firmware names be in DT" discussion.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: st_fdma: Firmware filename in DT?

[Lee Jones]

On Fri, 11 Sep 2015, Arnd Bergmann wrote:

> On Friday 11 September 2015 10:17:00 Lee Jones wrote:
> > <hijacking>
> > 
> > That doesn't work for middle-layer drivers such as Remoteproc, where
> > it doesn't have its own associated firmwares.  Remoteproc's job is
> > simply to load the firmware.  It doesn't care which version of the ABI
> > that particular binary uses, and has no reason to.  Ideally, I guess
> > the Remoteproc client should be providing the firmware name, but why
> > should the client care who or what was used to load the firmware?
> > 
> > </hijacking>
> > 
> 
> Does remoteproc use request_firmware() then?

Yes ...

> If not, it's irrelevant to this discussion.

... but even if it didn't, it would still be relevant, as it's a
"should firmware names be in DT" discussion.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: st_fdma: Firmware filename in DT?

[Peter Griffin]

Hi Lee,

On Fri, 11 Sep 2015, Lee Jones wrote:

> On Thu, 10 Sep 2015, Peter Griffin wrote:
> > On Mon, 07 Sep 2015, Daniel Thompson wrote:
> > > On 07/09/15 13:33, Arnd Bergmann wrote:
> > > >On Monday 07 September 2015 11:30:11 Daniel Thompson wrote:
> > > >>>>
> > > >>>>I would suggest "firmware-names" to be consistent with other naming
> > > >>>>convention and because their can be more than one (either at the same
> > > >>>>time or as fallback). It also leaves "firmware" available if we want
> > > >>>>to have phandle's to firmware embedded in the DT at some point later.
> > > >>>
> > > >>>Having list of strings would certainly make this more flexible than
> > > >>>just a single string, for the same reason as taking the list of
> > > >>>compatible values as the base, so +1 for "firmware-names" over "firmware".
> > > >>
> > > >>I was recently thinking about how DT filenames would interact with
> > > >>incompatible ABI changes to the firmware's register and/or wire protocol.
> > > >>
> > > >>Just for example we start with f/ware ABI A and we create a mainline
> > > >>kernel X that supported it.
> > > >>
> > > >>Vendor now releases a new firmware with ABI B and we update the mainline
> > > >>driver to support ABI A (to avoid breaking old userspaces) and B,
> > > >>eventually releasing kernel Y.
> > > >>
> > > >>The question now is how kernels X and Y should use the DT to generate
> > > >>the filename. It is very desirable that kernels X and Y use *different*
> > > >>filenames because otherwise a single userspace could not support the new
> > > >>feature *and* boot with both X and Y.
> > > >
> > > >Generally speaking, the kernel should shield user space from ABI
> > > >differences in the firmware and still provide the same user space ABI
> > > >(possibly emulated, when the newer firmware removes features of the
> > > >old one). Can you think of a case where a device firmware directly
> > > >defines the user ABI without having kernel visible changes?
> > > 
> > > I don't mean that the firmware ABI is exposed to userspace.
> > > 
> > > I mean if you use the same filename for both ABIs (in /lib/firmware)
> > > then kernel X with lose functionality if it is booted with a
> > > userspace that has updated to the new firmware (maybe even crash if
> > > I can't detect at runtime the version of the firmware is has
> > > loaded). That sort of thing is the pain for distros which support
> > > booting with multiple kernels (apt-get/dnf simply won't know when it
> > > is safe to update the firmware binary).
> > > 
> > > 
> > > >>Having lists of firmwares can certainly help solve this (providing the
> > > >>list can be used to allow a driver to select for an ABI is supports).
> > > >>However I afraid I find this example argues against having filenames in
> > > >>DT at all because it seems odd to me that, for kernel and userspace to
> > > >>adopt ABI B that must wait until the DT is updated to include support
> > > >>for ABI B. The hardware didn't change...
> > 
> > This is a really good point and one I'd not considered.
> > 
> > > >
> > > >This is not what I was thinking of for a list of file names: I would not
> > > >want a case where the kernel might pick between multiple incompatible
> > > >files without knowing what they are, especially if the differences
> > > >are ABI relevant.
> > > 
> > > Nor me.
> > > 
> > > Personally I prefer an "assertive" driver that imposes a filename
> > > for the firmwares it needs. Thus if there is an ABI change the
> > > driver writer can load a completely different firmware file without
> > > needing a DT update. That said I'm not especially invested either
> > > way.
> > > 
> > > However whatever solution is reached I would quite like to be able
> > > to boot kernels from either side of an ABI break from a single
> > > userspace (and get the new features when the kernel supports them).
> > 
> > So to summarise, I think given Daniels good points about ABI
> > compatability, and also David and Arnd PoV, it would be best to leave
> > the generation of the firmware filename to the individual driver and
> > not put the filename in DT.
> 
> <hijacking>
> 
> That doesn't work for middle-layer drivers such as Remoteproc, where
> it doesn't have its own associated firmwares.  Remoteproc's job is
> simply to load the firmware.  It doesn't care which version of the ABI
> that particular binary uses, and has no reason to.  Ideally, I guess
> the Remoteproc client should be providing the firmware name, but why
> should the client care who or what was used to load the firmware?

I believe you answered your own question...

In your case, I believe it should be the client driver which wishes to
use rproc to boot the co-processor which provides the name.

For example a v4l2 driver which wishes to use the st231-deltamu
co-processor, and requests rproc to boot that CPU with a supplied
firmware name. The v4l2 driver has the knowledge to know it wishes to
use the st231 for video decoding, and can provide a unique name based
on its compatible (what SoC etc) it is with an approriate suffix / prefix.

This nicely also handles the

"The hardware is identical, and different firmware is used to apply
   it in different ways."

which David pointed out.

Say you now wish to use that st231 for some other general
accelerator usecase, and have client driver X, then X can provide
a suitable unique name to rproc.

To accomodate that usecase with the filename in DT would require changing DT
to provide a new filename. Obviously the hardware hasn't changed, just the
way in which you wish to apply it.

This also solves the issue Daniel pointed out about the name being
sufficiently unique to allow multiple STi SoC's to use the same
userspace, which your current approach does not.

To answer your question, the client should care about the filename
because it is the client which wishes to use the services of the firimware
and is in a unique position to know what that firmware is called.
As you point out, rproc is not in that position.

regards,

Peter.

Re: st_fdma: Firmware filename in DT?

[Lee Jones]

On Fri, 11 Sep 2015, Peter Griffin wrote:
> On Fri, 11 Sep 2015, Lee Jones wrote:
> > On Thu, 10 Sep 2015, Peter Griffin wrote:
> > > On Mon, 07 Sep 2015, Daniel Thompson wrote:
> > > > On 07/09/15 13:33, Arnd Bergmann wrote:
> > > > >On Monday 07 September 2015 11:30:11 Daniel Thompson wrote:
> > > > >>>>
> > > > >>>>I would suggest "firmware-names" to be consistent with other naming
> > > > >>>>convention and because their can be more than one (either at the same
> > > > >>>>time or as fallback). It also leaves "firmware" available if we want
> > > > >>>>to have phandle's to firmware embedded in the DT at some point later.
> > > > >>>
> > > > >>>Having list of strings would certainly make this more flexible than
> > > > >>>just a single string, for the same reason as taking the list of
> > > > >>>compatible values as the base, so +1 for "firmware-names" over "firmware".
> > > > >>
> > > > >>I was recently thinking about how DT filenames would interact with
> > > > >>incompatible ABI changes to the firmware's register and/or wire protocol.
> > > > >>
> > > > >>Just for example we start with f/ware ABI A and we create a mainline
> > > > >>kernel X that supported it.
> > > > >>
> > > > >>Vendor now releases a new firmware with ABI B and we update the mainline
> > > > >>driver to support ABI A (to avoid breaking old userspaces) and B,
> > > > >>eventually releasing kernel Y.
> > > > >>
> > > > >>The question now is how kernels X and Y should use the DT to generate
> > > > >>the filename. It is very desirable that kernels X and Y use *different*
> > > > >>filenames because otherwise a single userspace could not support the new
> > > > >>feature *and* boot with both X and Y.
> > > > >
> > > > >Generally speaking, the kernel should shield user space from ABI
> > > > >differences in the firmware and still provide the same user space ABI
> > > > >(possibly emulated, when the newer firmware removes features of the
> > > > >old one). Can you think of a case where a device firmware directly
> > > > >defines the user ABI without having kernel visible changes?
> > > > 
> > > > I don't mean that the firmware ABI is exposed to userspace.
> > > > 
> > > > I mean if you use the same filename for both ABIs (in /lib/firmware)
> > > > then kernel X with lose functionality if it is booted with a
> > > > userspace that has updated to the new firmware (maybe even crash if
> > > > I can't detect at runtime the version of the firmware is has
> > > > loaded). That sort of thing is the pain for distros which support
> > > > booting with multiple kernels (apt-get/dnf simply won't know when it
> > > > is safe to update the firmware binary).
> > > > 
> > > > 
> > > > >>Having lists of firmwares can certainly help solve this (providing the
> > > > >>list can be used to allow a driver to select for an ABI is supports).
> > > > >>However I afraid I find this example argues against having filenames in
> > > > >>DT at all because it seems odd to me that, for kernel and userspace to
> > > > >>adopt ABI B that must wait until the DT is updated to include support
> > > > >>for ABI B. The hardware didn't change...
> > > 
> > > This is a really good point and one I'd not considered.
> > > 
> > > > >
> > > > >This is not what I was thinking of for a list of file names: I would not
> > > > >want a case where the kernel might pick between multiple incompatible
> > > > >files without knowing what they are, especially if the differences
> > > > >are ABI relevant.
> > > > 
> > > > Nor me.
> > > > 
> > > > Personally I prefer an "assertive" driver that imposes a filename
> > > > for the firmwares it needs. Thus if there is an ABI change the
> > > > driver writer can load a completely different firmware file without
> > > > needing a DT update. That said I'm not especially invested either
> > > > way.
> > > > 
> > > > However whatever solution is reached I would quite like to be able
> > > > to boot kernels from either side of an ABI break from a single
> > > > userspace (and get the new features when the kernel supports them).
> > > 
> > > So to summarise, I think given Daniels good points about ABI
> > > compatability, and also David and Arnd PoV, it would be best to leave
> > > the generation of the firmware filename to the individual driver and
> > > not put the filename in DT.
> > 
> > <hijacking>
> > 
> > That doesn't work for middle-layer drivers such as Remoteproc, where
> > it doesn't have its own associated firmwares.  Remoteproc's job is
> > simply to load the firmware.  It doesn't care which version of the ABI
> > that particular binary uses, and has no reason to.  Ideally, I guess
> > the Remoteproc client should be providing the firmware name, but why
> > should the client care who or what was used to load the firmware?
> 
> I believe you answered your own question...
> 
> In your case, I believe it should be the client driver which wishes to
> use rproc to boot the co-processor which provides the name.
> 
> For example a v4l2 driver which wishes to use the st231-deltamu
> co-processor, and requests rproc to boot that CPU with a supplied
> firmware name. The v4l2 driver has the knowledge to know it wishes to
> use the st231 for video decoding, and can provide a unique name based
> on its compatible (what SoC etc) it is with an approriate suffix / prefix.
> 
> This nicely also handles the
> 
> "The hardware is identical, and different firmware is used to apply
>    it in different ways."
> 
> which David pointed out.
> 
> Say you now wish to use that st231 for some other general
> accelerator usecase, and have client driver X, then X can provide
> a suitable unique name to rproc.
> 
> To accomodate that usecase with the filename in DT would require changing DT
> to provide a new filename. Obviously the hardware hasn't changed, just the
> way in which you wish to apply it.
> 
> This also solves the issue Daniel pointed out about the name being
> sufficiently unique to allow multiple STi SoC's to use the same
> userspace, which your current approach does not.
> 
> To answer your question, the client should care about the filename
> because it is the client which wishes to use the services of the firimware
> and is in a unique position to know what that firmware is called.
> As you point out, rproc is not in that position.

You're preaching to the converted.  I agree with all of this, which is
why I said:

"Ideally, I guess the Remoteproc client should be providing the
firmware name"

... but you missed the critical part of my query:

"but why should the client care who or what was used to load the
firmware?"

At the moment the client has no other way (besides DT) to tell
Remoteproc (or any other firmware loader) which firmware binary to
load.  To solve this, we would have to call load_firmware(name)
[which doesn't exist yet], from the client, into either the core or
the vendor driver.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: st_fdma: Firmware filename in DT?

[Lee Jones]

On Fri, 11 Sep 2015, Peter Griffin wrote:
> On Fri, 11 Sep 2015, Lee Jones wrote:
> > On Thu, 10 Sep 2015, Peter Griffin wrote:
> > > On Mon, 07 Sep 2015, Daniel Thompson wrote:
> > > > On 07/09/15 13:33, Arnd Bergmann wrote:
> > > > >On Monday 07 September 2015 11:30:11 Daniel Thompson wrote:
> > > > >>>>
> > > > >>>>I would suggest "firmware-names" to be consistent with other naming
> > > > >>>>convention and because their can be more than one (either at the same
> > > > >>>>time or as fallback). It also leaves "firmware" available if we want
> > > > >>>>to have phandle's to firmware embedded in the DT at some point later.
> > > > >>>
> > > > >>>Having list of strings would certainly make this more flexible than
> > > > >>>just a single string, for the same reason as taking the list of
> > > > >>>compatible values as the base, so +1 for "firmware-names" over "firmware".
> > > > >>
> > > > >>I was recently thinking about how DT filenames would interact with
> > > > >>incompatible ABI changes to the firmware's register and/or wire protocol.
> > > > >>
> > > > >>Just for example we start with f/ware ABI A and we create a mainline
> > > > >>kernel X that supported it.
> > > > >>
> > > > >>Vendor now releases a new firmware with ABI B and we update the mainline
> > > > >>driver to support ABI A (to avoid breaking old userspaces) and B,
> > > > >>eventually releasing kernel Y.
> > > > >>
> > > > >>The question now is how kernels X and Y should use the DT to generate
> > > > >>the filename. It is very desirable that kernels X and Y use *different*
> > > > >>filenames because otherwise a single userspace could not support the new
> > > > >>feature *and* boot with both X and Y.
> > > > >
> > > > >Generally speaking, the kernel should shield user space from ABI
> > > > >differences in the firmware and still provide the same user space ABI
> > > > >(possibly emulated, when the newer firmware removes features of the
> > > > >old one). Can you think of a case where a device firmware directly
> > > > >defines the user ABI without having kernel visible changes?
> > > > 
> > > > I don't mean that the firmware ABI is exposed to userspace.
> > > > 
> > > > I mean if you use the same filename for both ABIs (in /lib/firmware)
> > > > then kernel X with lose functionality if it is booted with a
> > > > userspace that has updated to the new firmware (maybe even crash if
> > > > I can't detect at runtime the version of the firmware is has
> > > > loaded). That sort of thing is the pain for distros which support
> > > > booting with multiple kernels (apt-get/dnf simply won't know when it
> > > > is safe to update the firmware binary).
> > > > 
> > > > 
> > > > >>Having lists of firmwares can certainly help solve this (providing the
> > > > >>list can be used to allow a driver to select for an ABI is supports).
> > > > >>However I afraid I find this example argues against having filenames in
> > > > >>DT at all because it seems odd to me that, for kernel and userspace to
> > > > >>adopt ABI B that must wait until the DT is updated to include support
> > > > >>for ABI B. The hardware didn't change...
> > > 
> > > This is a really good point and one I'd not considered.
> > > 
> > > > >
> > > > >This is not what I was thinking of for a list of file names: I would not
> > > > >want a case where the kernel might pick between multiple incompatible
> > > > >files without knowing what they are, especially if the differences
> > > > >are ABI relevant.
> > > > 
> > > > Nor me.
> > > > 
> > > > Personally I prefer an "assertive" driver that imposes a filename
> > > > for the firmwares it needs. Thus if there is an ABI change the
> > > > driver writer can load a completely different firmware file without
> > > > needing a DT update. That said I'm not especially invested either
> > > > way.
> > > > 
> > > > However whatever solution is reached I would quite like to be able
> > > > to boot kernels from either side of an ABI break from a single
> > > > userspace (and get the new features when the kernel supports them).
> > > 
> > > So to summarise, I think given Daniels good points about ABI
> > > compatability, and also David and Arnd PoV, it would be best to leave
> > > the generation of the firmware filename to the individual driver and
> > > not put the filename in DT.
> > 
> > <hijacking>
> > 
> > That doesn't work for middle-layer drivers such as Remoteproc, where
> > it doesn't have its own associated firmwares.  Remoteproc's job is
> > simply to load the firmware.  It doesn't care which version of the ABI
> > that particular binary uses, and has no reason to.  Ideally, I guess
> > the Remoteproc client should be providing the firmware name, but why
> > should the client care who or what was used to load the firmware?
> 
> I believe you answered your own question...
> 
> In your case, I believe it should be the client driver which wishes to
> use rproc to boot the co-processor which provides the name.
> 
> For example a v4l2 driver which wishes to use the st231-deltamu
> co-processor, and requests rproc to boot that CPU with a supplied
> firmware name. The v4l2 driver has the knowledge to know it wishes to
> use the st231 for video decoding, and can provide a unique name based
> on its compatible (what SoC etc) it is with an approriate suffix / prefix.
> 
> This nicely also handles the
> 
> "The hardware is identical, and different firmware is used to apply
>    it in different ways."
> 
> which David pointed out.
> 
> Say you now wish to use that st231 for some other general
> accelerator usecase, and have client driver X, then X can provide
> a suitable unique name to rproc.
> 
> To accomodate that usecase with the filename in DT would require changing DT
> to provide a new filename. Obviously the hardware hasn't changed, just the
> way in which you wish to apply it.
> 
> This also solves the issue Daniel pointed out about the name being
> sufficiently unique to allow multiple STi SoC's to use the same
> userspace, which your current approach does not.
> 
> To answer your question, the client should care about the filename
> because it is the client which wishes to use the services of the firimware
> and is in a unique position to know what that firmware is called.
> As you point out, rproc is not in that position.

You're preaching to the converted.  I agree with all of this, which is
why I said:

"Ideally, I guess the Remoteproc client should be providing the
firmware name"

... but you missed the critical part of my query:

"but why should the client care who or what was used to load the
firmware?"

At the moment the client has no other way (besides DT) to tell
Remoteproc (or any other firmware loader) which firmware binary to
load.  To solve this, we would have to call load_firmware(name)
[which doesn't exist yet], from the client, into either the core or
the vendor driver.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: st_fdma: Firmware filename in DT?

[Peter Griffin]

Hi Lee,

On Fri, 11 Sep 2015, Lee Jones wrote:

> On Fri, 11 Sep 2015, Peter Griffin wrote:
> > On Fri, 11 Sep 2015, Lee Jones wrote:
> > > On Thu, 10 Sep 2015, Peter Griffin wrote:
> > > > On Mon, 07 Sep 2015, Daniel Thompson wrote:
> > > > > On 07/09/15 13:33, Arnd Bergmann wrote:
> > > > > >On Monday 07 September 2015 11:30:11 Daniel Thompson wrote:
> > > > > >>>>
> > > > > >>>>I would suggest "firmware-names" to be consistent with other naming
> > > > > >>>>convention and because their can be more than one (either at the same
> > > > > >>>>time or as fallback). It also leaves "firmware" available if we want
> > > > > >>>>to have phandle's to firmware embedded in the DT at some point later.
> > > > > >>>
> > > > > >>>Having list of strings would certainly make this more flexible than
> > > > > >>>just a single string, for the same reason as taking the list of
> > > > > >>>compatible values as the base, so +1 for "firmware-names" over "firmware".
> > > > > >>
> > > > > >>I was recently thinking about how DT filenames would interact with
> > > > > >>incompatible ABI changes to the firmware's register and/or wire protocol.
> > > > > >>
> > > > > >>Just for example we start with f/ware ABI A and we create a mainline
> > > > > >>kernel X that supported it.
> > > > > >>
> > > > > >>Vendor now releases a new firmware with ABI B and we update the mainline
> > > > > >>driver to support ABI A (to avoid breaking old userspaces) and B,
> > > > > >>eventually releasing kernel Y.
> > > > > >>
> > > > > >>The question now is how kernels X and Y should use the DT to generate
> > > > > >>the filename. It is very desirable that kernels X and Y use *different*
> > > > > >>filenames because otherwise a single userspace could not support the new
> > > > > >>feature *and* boot with both X and Y.
> > > > > >
> > > > > >Generally speaking, the kernel should shield user space from ABI
> > > > > >differences in the firmware and still provide the same user space ABI
> > > > > >(possibly emulated, when the newer firmware removes features of the
> > > > > >old one). Can you think of a case where a device firmware directly
> > > > > >defines the user ABI without having kernel visible changes?
> > > > > 
> > > > > I don't mean that the firmware ABI is exposed to userspace.
> > > > > 
> > > > > I mean if you use the same filename for both ABIs (in /lib/firmware)
> > > > > then kernel X with lose functionality if it is booted with a
> > > > > userspace that has updated to the new firmware (maybe even crash if
> > > > > I can't detect at runtime the version of the firmware is has
> > > > > loaded). That sort of thing is the pain for distros which support
> > > > > booting with multiple kernels (apt-get/dnf simply won't know when it
> > > > > is safe to update the firmware binary).
> > > > > 
> > > > > 
> > > > > >>Having lists of firmwares can certainly help solve this (providing the
> > > > > >>list can be used to allow a driver to select for an ABI is supports).
> > > > > >>However I afraid I find this example argues against having filenames in
> > > > > >>DT at all because it seems odd to me that, for kernel and userspace to
> > > > > >>adopt ABI B that must wait until the DT is updated to include support
> > > > > >>for ABI B. The hardware didn't change...
> > > > 
> > > > This is a really good point and one I'd not considered.
> > > > 
> > > > > >
> > > > > >This is not what I was thinking of for a list of file names: I would not
> > > > > >want a case where the kernel might pick between multiple incompatible
> > > > > >files without knowing what they are, especially if the differences
> > > > > >are ABI relevant.
> > > > > 
> > > > > Nor me.
> > > > > 
> > > > > Personally I prefer an "assertive" driver that imposes a filename
> > > > > for the firmwares it needs. Thus if there is an ABI change the
> > > > > driver writer can load a completely different firmware file without
> > > > > needing a DT update. That said I'm not especially invested either
> > > > > way.
> > > > > 
> > > > > However whatever solution is reached I would quite like to be able
> > > > > to boot kernels from either side of an ABI break from a single
> > > > > userspace (and get the new features when the kernel supports them).
> > > > 
> > > > So to summarise, I think given Daniels good points about ABI
> > > > compatability, and also David and Arnd PoV, it would be best to leave
> > > > the generation of the firmware filename to the individual driver and
> > > > not put the filename in DT.
> > > 
> > > <hijacking>
> > > 
> > > That doesn't work for middle-layer drivers such as Remoteproc, where
> > > it doesn't have its own associated firmwares.  Remoteproc's job is
> > > simply to load the firmware.  It doesn't care which version of the ABI
> > > that particular binary uses, and has no reason to.  Ideally, I guess
> > > the Remoteproc client should be providing the firmware name, but why
> > > should the client care who or what was used to load the firmware?
> > 
> > I believe you answered your own question...
> > 
> > In your case, I believe it should be the client driver which wishes to
> > use rproc to boot the co-processor which provides the name.
> > 
> > For example a v4l2 driver which wishes to use the st231-deltamu
> > co-processor, and requests rproc to boot that CPU with a supplied
> > firmware name. The v4l2 driver has the knowledge to know it wishes to
> > use the st231 for video decoding, and can provide a unique name based
> > on its compatible (what SoC etc) it is with an approriate suffix / prefix.
> > 
> > This nicely also handles the
> > 
> > "The hardware is identical, and different firmware is used to apply
> >    it in different ways."
> > 
> > which David pointed out.
> > 
> > Say you now wish to use that st231 for some other general
> > accelerator usecase, and have client driver X, then X can provide
> > a suitable unique name to rproc.
> > 
> > To accomodate that usecase with the filename in DT would require changing DT
> > to provide a new filename. Obviously the hardware hasn't changed, just the
> > way in which you wish to apply it.
> > 
> > This also solves the issue Daniel pointed out about the name being
> > sufficiently unique to allow multiple STi SoC's to use the same
> > userspace, which your current approach does not.
> > 
> > To answer your question, the client should care about the filename
> > because it is the client which wishes to use the services of the firimware
> > and is in a unique position to know what that firmware is called.
> > As you point out, rproc is not in that position.
> 
> You're preaching to the converted.  I agree with all of this, which is
> why I said:

Oh I thought you were un-converted as you said it wouldn't work for rproc.

> 
> "Ideally, I guess the Remoteproc client should be providing the
> firmware name"
> 
> ... but you missed the critical part of my query:
> 
> "but why should the client care who or what was used to load the
> firmware?"

Most drivers in the kernel if they need a firmware call request_firmware
or request_firmware_nowait directly, i.e. they care about loading the
firmware that they need and call the correct kernel API to do so.

If your client driver wishes to use rproc to load the firmware instead of
request_firmware directly, then it seems obvious that it needs to care
about rproc API's so it can call them.

> 
> At the moment the client has no other way (besides DT) to tell
> Remoteproc (or any other firmware loader) which firmware binary to
> load.

Um yes it does, using the compatible strings like we've already
discussed to generate a unique filename.

>
>  To solve this, we would have to call load_firmware(name)
> [which doesn't exist yet], from the client, into either the core or
> the vendor driver.

I'm not sure what your getting at here (or maybe I've misunderstood you)
anyways...

The API for loading a firmware in the kernel is request_firmware, or
request_firmware_nowait, and you pass it the name of the firmware you
wish to load. So the API exists, and is used throughout the kernel.

The client driver then generates a unique name based on the compatible
and calls request_firmware(name).

>From what I can see rproc then adds another level of abstraction on
top of the request_firmware API. But the ability to pass
a name to request_firmware still exists. Maybe it will require changes
in rproc to propogate the name down properly to the request_firmware call,
but that is just a rproc implementation detail.

regards,

Peter.

Re: st_fdma: Firmware filename in DT?

[Peter Griffin]

Hi Lee,

On Fri, 11 Sep 2015, Lee Jones wrote:

> On Fri, 11 Sep 2015, Peter Griffin wrote:
> > On Fri, 11 Sep 2015, Lee Jones wrote:
> > > On Thu, 10 Sep 2015, Peter Griffin wrote:
> > > > On Mon, 07 Sep 2015, Daniel Thompson wrote:
> > > > > On 07/09/15 13:33, Arnd Bergmann wrote:
> > > > > >On Monday 07 September 2015 11:30:11 Daniel Thompson wrote:
> > > > > >>>>
> > > > > >>>>I would suggest "firmware-names" to be consistent with other naming
> > > > > >>>>convention and because their can be more than one (either at the same
> > > > > >>>>time or as fallback). It also leaves "firmware" available if we want
> > > > > >>>>to have phandle's to firmware embedded in the DT at some point later.
> > > > > >>>
> > > > > >>>Having list of strings would certainly make this more flexible than
> > > > > >>>just a single string, for the same reason as taking the list of
> > > > > >>>compatible values as the base, so +1 for "firmware-names" over "firmware".
> > > > > >>
> > > > > >>I was recently thinking about how DT filenames would interact with
> > > > > >>incompatible ABI changes to the firmware's register and/or wire protocol.
> > > > > >>
> > > > > >>Just for example we start with f/ware ABI A and we create a mainline
> > > > > >>kernel X that supported it.
> > > > > >>
> > > > > >>Vendor now releases a new firmware with ABI B and we update the mainline
> > > > > >>driver to support ABI A (to avoid breaking old userspaces) and B,
> > > > > >>eventually releasing kernel Y.
> > > > > >>
> > > > > >>The question now is how kernels X and Y should use the DT to generate
> > > > > >>the filename. It is very desirable that kernels X and Y use *different*
> > > > > >>filenames because otherwise a single userspace could not support the new
> > > > > >>feature *and* boot with both X and Y.
> > > > > >
> > > > > >Generally speaking, the kernel should shield user space from ABI
> > > > > >differences in the firmware and still provide the same user space ABI
> > > > > >(possibly emulated, when the newer firmware removes features of the
> > > > > >old one). Can you think of a case where a device firmware directly
> > > > > >defines the user ABI without having kernel visible changes?
> > > > > 
> > > > > I don't mean that the firmware ABI is exposed to userspace.
> > > > > 
> > > > > I mean if you use the same filename for both ABIs (in /lib/firmware)
> > > > > then kernel X with lose functionality if it is booted with a
> > > > > userspace that has updated to the new firmware (maybe even crash if
> > > > > I can't detect at runtime the version of the firmware is has
> > > > > loaded). That sort of thing is the pain for distros which support
> > > > > booting with multiple kernels (apt-get/dnf simply won't know when it
> > > > > is safe to update the firmware binary).
> > > > > 
> > > > > 
> > > > > >>Having lists of firmwares can certainly help solve this (providing the
> > > > > >>list can be used to allow a driver to select for an ABI is supports).
> > > > > >>However I afraid I find this example argues against having filenames in
> > > > > >>DT at all because it seems odd to me that, for kernel and userspace to
> > > > > >>adopt ABI B that must wait until the DT is updated to include support
> > > > > >>for ABI B. The hardware didn't change...
> > > > 
> > > > This is a really good point and one I'd not considered.
> > > > 
> > > > > >
> > > > > >This is not what I was thinking of for a list of file names: I would not
> > > > > >want a case where the kernel might pick between multiple incompatible
> > > > > >files without knowing what they are, especially if the differences
> > > > > >are ABI relevant.
> > > > > 
> > > > > Nor me.
> > > > > 
> > > > > Personally I prefer an "assertive" driver that imposes a filename
> > > > > for the firmwares it needs. Thus if there is an ABI change the
> > > > > driver writer can load a completely different firmware file without
> > > > > needing a DT update. That said I'm not especially invested either
> > > > > way.
> > > > > 
> > > > > However whatever solution is reached I would quite like to be able
> > > > > to boot kernels from either side of an ABI break from a single
> > > > > userspace (and get the new features when the kernel supports them).
> > > > 
> > > > So to summarise, I think given Daniels good points about ABI
> > > > compatability, and also David and Arnd PoV, it would be best to leave
> > > > the generation of the firmware filename to the individual driver and
> > > > not put the filename in DT.
> > > 
> > > <hijacking>
> > > 
> > > That doesn't work for middle-layer drivers such as Remoteproc, where
> > > it doesn't have its own associated firmwares.  Remoteproc's job is
> > > simply to load the firmware.  It doesn't care which version of the ABI
> > > that particular binary uses, and has no reason to.  Ideally, I guess
> > > the Remoteproc client should be providing the firmware name, but why
> > > should the client care who or what was used to load the firmware?
> > 
> > I believe you answered your own question...
> > 
> > In your case, I believe it should be the client driver which wishes to
> > use rproc to boot the co-processor which provides the name.
> > 
> > For example a v4l2 driver which wishes to use the st231-deltamu
> > co-processor, and requests rproc to boot that CPU with a supplied
> > firmware name. The v4l2 driver has the knowledge to know it wishes to
> > use the st231 for video decoding, and can provide a unique name based
> > on its compatible (what SoC etc) it is with an approriate suffix / prefix.
> > 
> > This nicely also handles the
> > 
> > "The hardware is identical, and different firmware is used to apply
> >    it in different ways."
> > 
> > which David pointed out.
> > 
> > Say you now wish to use that st231 for some other general
> > accelerator usecase, and have client driver X, then X can provide
> > a suitable unique name to rproc.
> > 
> > To accomodate that usecase with the filename in DT would require changing DT
> > to provide a new filename. Obviously the hardware hasn't changed, just the
> > way in which you wish to apply it.
> > 
> > This also solves the issue Daniel pointed out about the name being
> > sufficiently unique to allow multiple STi SoC's to use the same
> > userspace, which your current approach does not.
> > 
> > To answer your question, the client should care about the filename
> > because it is the client which wishes to use the services of the firimware
> > and is in a unique position to know what that firmware is called.
> > As you point out, rproc is not in that position.
> 
> You're preaching to the converted.  I agree with all of this, which is
> why I said:

Oh I thought you were un-converted as you said it wouldn't work for rproc.

> 
> "Ideally, I guess the Remoteproc client should be providing the
> firmware name"
> 
> ... but you missed the critical part of my query:
> 
> "but why should the client care who or what was used to load the
> firmware?"

Most drivers in the kernel if they need a firmware call request_firmware
or request_firmware_nowait directly, i.e. they care about loading the
firmware that they need and call the correct kernel API to do so.

If your client driver wishes to use rproc to load the firmware instead of
request_firmware directly, then it seems obvious that it needs to care
about rproc API's so it can call them.

> 
> At the moment the client has no other way (besides DT) to tell
> Remoteproc (or any other firmware loader) which firmware binary to
> load.

Um yes it does, using the compatible strings like we've already
discussed to generate a unique filename.

>
>  To solve this, we would have to call load_firmware(name)
> [which doesn't exist yet], from the client, into either the core or
> the vendor driver.

I'm not sure what your getting at here (or maybe I've misunderstood you)
anyways...

The API for loading a firmware in the kernel is request_firmware, or
request_firmware_nowait, and you pass it the name of the firmware you
wish to load. So the API exists, and is used throughout the kernel.

The client driver then generates a unique name based on the compatible
and calls request_firmware(name).

From what I can see rproc then adds another level of abstraction on
top of the request_firmware API. But the ability to pass
a name to request_firmware still exists. Maybe it will require changes
in rproc to propogate the name down properly to the request_firmware call,
but that is just a rproc implementation detail.

regards,

Peter.