From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 Sender: wlosh@bsdimp.com In-Reply-To: <201509051058.54229.arnd@arndb.de> References: <20150903144944.GC7093@griffinp-ThinkPad-X1-Carbon-2nd> <201509041504.38412.arnd@arndb.de> <201509051058.54229.arnd@arndb.de> Date: Sat, 5 Sep 2015 15:06:35 -0600 Message-ID: Subject: Re: st_fdma: Firmware filename in DT? From: Warner Losh Content-Type: multipart/alternative; boundary=001a11354d5c3aab81051f0664de To: Arnd Bergmann Cc: Lee Jones , Peter Griffin , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Vinod Koul , "devicetree@vger.kernel.org" , Maxime Coquelin , Patrice Chotard , Ludovic Barre , "devicetree-spec@vger.kernel.org" List-ID: --001a11354d5c3aab81051f0664de Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Sat, Sep 5, 2015 at 2:58 AM, Arnd Bergmann wrote: > On Friday 04 September 2015, Warner Losh wrote: > > > On Sep 4, 2015, at 7:04 AM, 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 Purpo= se > > >>>> Co-Processor RemoteProc driver has a corresponding node called > > >>>> 'st231-gp0@40000000'. RemoteProc adds an 'rproc-' prefix and a > '-fw' > > >>>> suffix and et voil=C3=A0, 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 =3D=3D 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 appropriat= e. > > > > Encoding the driver name is not OS independent. It fosters the > > impression that DT isn=E2=80=99t 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 specif= ic > > > 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=E2=80=99t 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 > > =E2=80=9Cimages=E2=80=9D that are loaded into one hardware block, but a= ctually 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 othe= r > 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 th= e > more specific one, if any, or be provided by the user. As an example, > you can have > > compatible =3D "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 --001a11354d5c3aab81051f0664de Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Sat, Sep 5, 2015 at 2:58 AM, Arnd Bergmann <arnd@arndb.de> wrote:
On Friday 04 Se= ptember 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 de= rive the firmware
> >>>> name from the 'node name'.=C2=A0 For instance= , our zeroth General Purpose
> >>>> Co-Processor RemoteProc driver has a corresponding no= de called
> >>>> 'st231-gp0@40000000'.=C2=A0 RemoteProc adds a= n 'rproc-' prefix and a '-fw'
> >>>> suffix and et voil=C3=A0, we load file:
> >>>>
> >>>>=C2=A0 lib/firmware/rproc-st231-gp0-fw
> >>>
> >>> IMO deriving from the node name seems fragile. Also impos= ing 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 remot= eproc subsys core
> >>> is currently working. It seems a generic DT firmware bind= ing would actually
> >>> be most useful for the remoteproc subsystem.
> >>
> >> The "rproc-%s-fw", where %s =3D=3D driver name, is = only a fall-back.=C2=A0 The
> >> RProc driver is welcome to supply a different firmware name i= f it
> >> desires.=C2=A0 This is where I think a generic 'firmware&= #39; 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 approp= riate.
>
> Encoding the driver name is not OS independent. It fosters the
> impression that DT isn=E2=80=99t 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,<= br> e.g.

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0$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 bas= ing
> > the firmware name on the "compatible" property strings.= That property
> > already contains a hierarchical list of models, which makes it pa= rticularly
> > easy to have firmware files for specific models or those that are= shared
> > across multiple variations if necessary. Just ask for the most sp= ecific
> > compatible string first and try the more specific compatible stri= ngs
> > (with an appropriate prefix and/or postfix added by the driver) u= ntil
> > a file is found.
>
> I think this is a horrible policy. It is an ugly kludge that is fragil= e to change.
> While it sounds cool, I don=E2=80=99t think it is really a viable one.= It requires
> different compatibility names for different bits of hardware that migh= t otherwise
> be the same just to get different firmware. Sometimes this may be OK,<= br> > but it does seem needlessly limiting for systems that may have firmwar= e
> =E2=80=9Cimages=E2=80=9D 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 har= dware
but also how it is getting used. This is the same as what we do with other<= br> programmable hardware, such as PHY controllers that can be either PCIe
or USB3 depending on some setting. The driver would match on the more gener= ic
property that identifies the hardware, while the firmware can match on the<= br> more specific one, if any, or be provided by the user. As an example,
you can have

=C2=A0 =C2=A0 =C2=A0 =C2=A0 compatible =3D "fwmaker,firmwaretype"= , "hardwaremaker,thisdevice-instance2",
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "hardwaremaker= ,thisdevice-version-3.1", "hardwaremaker,thisdevice";

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

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

I understood what you were proposing. It wasn't a lack of understandi= ng
that lead me to the opinion that it was a horrible policy. Des= pite your
lengthy explanation, you did nothing to address the ver= y simple and common
use case of having identical hardware that no= ne-the-less needs different
firmware which was my basis of the ho= rrible brand I put on this policy.

Also, firmware = comes from vendors generally named something other
than FDT compa= t 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 plac= e 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 fo= r this node so that the system behaves
as expect in the DT is Y.&= #39;

Warner
--001a11354d5c3aab81051f0664de--