All of lore.kernel.org
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale.com>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: Mitch Bradley <wmb@firmworks.com>,
	linuxppc-dev@ozlabs.org, devicetree-discuss@lists.ozlabs.org,
	Timur Tabi <timur@freescale.com>
Subject: Re: [PATCH] powerpc/fsl: add device tree binding for QE firmware
Date: Thu, 25 Mar 2010 14:53:56 -0500	[thread overview]
Message-ID: <4BABBF54.3010104@freescale.com> (raw)
In-Reply-To: <fa686aa41003251035h3574a2bahce183ca6056e5f7e@mail.gmail.com>

Grant Likely wrote:
> On Thu, Mar 25, 2010 at 11:03 AM, Timur Tabi <timur@freescale.com> wrote:
>> Grant Likely wrote:
>>> For indirect firmware, create a /chosen/firmware node.  Don't add a
>>> compatible property,
>> Oh, I don't like that idea at all.  The compatible property is useful for me to know *how* to parse the binary blob.
> 
> Compatible is for devices.

Compatible is for matching.  Who cares what category the thing being 
matched is in?  What is the definition of a device, and why does it matter?

> This is not a device.  Drivers cannot bind
> against it.  Use a different mechanism if you have metadata about the
> blob.  If your driver doesn't know how to validate its own firmware
> blobs, then you've got bigger problems.

One could also say, if your hardware can't be probed at runtime, you've 
got bigger problems. :-)

What's wrong with an indication of what type of "thing" a node is 
supposed to be?  There could be multiple microcode formats, for example.

I don't know that it's strictly necessary in this case --  it looks like 
there is a magic number in the firmware blob -- but I don't understand 
the objection as a matter of principle.  These device tree discussions 
have a tendency to get awfully bikesheddy.

>>>  Put each firmware blob into a separate property, and make
>>> the names reasonable (ie. mpc<blah>-qe-firmware).  Have the QE
>>> reference the firmware blob by property name.
>> I don't like the idea of using the property name as a pseudo-compatible string.
> 
> It's a name, not a pseudo compatible string, and your device node will
> explicitly reference it by name.  There is not backwards compatibility
> or fuzzy binding issues at play here.

There is a forward compatibility issue, in that we'll have to update the 
code with every new mpc<blah> (or p<blah>rev<n>) that comes along.

Or are we supposed to pick some random chip to request the firmware for, 
like with compatibles?  What would be the point?  This isn't being used 
to bind a driver.

> It is a way for your driver
> node to state, "I want *that exact* firmware blob".

The driver wants the firmware blob that the device tree provides.  The 
device tree knows better than the driver, being that the device tree is 
the describer of the hardware.

> You could make
> the property name "george" 

If "george" is fine, then so is "fsl,firmware".  Maybe "fsl,qe-firmware".

> and it would still be completely clear (if
> a little weird) because all the references are contained within the
> tree.

How are the references contained within the tree?  The driver has to 
know which property to read.

-Scott

WARNING: multiple messages have this Message-ID (diff)
From: Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
To: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A@public.gmane.org,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	Timur Tabi <timur-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Subject: Re: [PATCH] powerpc/fsl: add device tree binding for QE firmware
Date: Thu, 25 Mar 2010 14:53:56 -0500	[thread overview]
Message-ID: <4BABBF54.3010104@freescale.com> (raw)
In-Reply-To: <fa686aa41003251035h3574a2bahce183ca6056e5f7e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Grant Likely wrote:
> On Thu, Mar 25, 2010 at 11:03 AM, Timur Tabi <timur-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
>> Grant Likely wrote:
>>> For indirect firmware, create a /chosen/firmware node.  Don't add a
>>> compatible property,
>> Oh, I don't like that idea at all.  The compatible property is useful for me to know *how* to parse the binary blob.
> 
> Compatible is for devices.

Compatible is for matching.  Who cares what category the thing being 
matched is in?  What is the definition of a device, and why does it matter?

> This is not a device.  Drivers cannot bind
> against it.  Use a different mechanism if you have metadata about the
> blob.  If your driver doesn't know how to validate its own firmware
> blobs, then you've got bigger problems.

One could also say, if your hardware can't be probed at runtime, you've 
got bigger problems. :-)

What's wrong with an indication of what type of "thing" a node is 
supposed to be?  There could be multiple microcode formats, for example.

I don't know that it's strictly necessary in this case --  it looks like 
there is a magic number in the firmware blob -- but I don't understand 
the objection as a matter of principle.  These device tree discussions 
have a tendency to get awfully bikesheddy.

>>>  Put each firmware blob into a separate property, and make
>>> the names reasonable (ie. mpc<blah>-qe-firmware).  Have the QE
>>> reference the firmware blob by property name.
>> I don't like the idea of using the property name as a pseudo-compatible string.
> 
> It's a name, not a pseudo compatible string, and your device node will
> explicitly reference it by name.  There is not backwards compatibility
> or fuzzy binding issues at play here.

There is a forward compatibility issue, in that we'll have to update the 
code with every new mpc<blah> (or p<blah>rev<n>) that comes along.

Or are we supposed to pick some random chip to request the firmware for, 
like with compatibles?  What would be the point?  This isn't being used 
to bind a driver.

> It is a way for your driver
> node to state, "I want *that exact* firmware blob".

The driver wants the firmware blob that the device tree provides.  The 
device tree knows better than the driver, being that the device tree is 
the describer of the hardware.

> You could make
> the property name "george" 

If "george" is fine, then so is "fsl,firmware".  Maybe "fsl,qe-firmware".

> and it would still be completely clear (if
> a little weird) because all the references are contained within the
> tree.

How are the references contained within the tree?  The driver has to 
know which property to read.

-Scott

  parent reply	other threads:[~2010-03-25 19:54 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-23 21:42 [PATCH] powerpc/fsl: add device tree binding for QE firmware Timur Tabi
2010-03-23 21:42 ` Timur Tabi
2010-03-24  6:07 ` Grant Likely
2010-03-24  6:07   ` Grant Likely
2010-03-24 12:05   ` Timur Tabi
2010-03-24 12:05     ` Timur Tabi
2010-03-24 17:00     ` Segher Boessenkool
2010-03-24 17:00       ` Segher Boessenkool
2010-03-24 17:07       ` Grant Likely
2010-03-24 17:07         ` Grant Likely
2010-03-24 17:31         ` Timur Tabi
2010-03-24 17:31           ` Timur Tabi
2010-03-24 18:10           ` Grant Likely
2010-03-24 18:10             ` Grant Likely
2010-03-24 18:21             ` Mitch Bradley
2010-03-24 18:21               ` Mitch Bradley
2010-03-24 18:25             ` Timur Tabi
2010-03-24 18:24           ` M. Warner Losh
2010-03-24 18:31             ` Timur Tabi
2010-03-24 18:31               ` Timur Tabi
2010-03-25  1:49           ` Segher Boessenkool
2010-03-25  1:49             ` Segher Boessenkool
2010-03-25 14:42             ` Timur Tabi
2010-03-25 14:42               ` Timur Tabi
2010-03-25 16:10               ` Grant Likely
2010-03-25 16:10                 ` Grant Likely
2010-03-25 16:34                 ` Scott Wood
2010-03-25 16:34                   ` Scott Wood
2010-03-25 16:46                   ` Timur Tabi
2010-03-25 16:46                     ` Timur Tabi
2010-03-26 18:23                     ` Rafal Jaworowski
2010-03-26 18:23                       ` Rafal Jaworowski
2010-03-25 23:53               ` M. Warner Losh
2010-03-25 23:53                 ` M. Warner Losh
2010-03-26  0:22                 ` Timur Tabi
2010-03-26  0:22                   ` Timur Tabi
2010-03-25 15:16             ` Scott Wood
2010-03-25 15:16               ` Scott Wood
2010-03-25 15:29               ` Mitch Bradley
2010-03-25 15:29                 ` Mitch Bradley
2010-03-25 16:16                 ` Grant Likely
2010-03-25 16:16                   ` Grant Likely
2010-03-25 16:36                   ` Timur Tabi
2010-03-25 16:36                     ` Timur Tabi
2010-03-25 16:50                     ` Scott Wood
2010-03-25 16:50                       ` Scott Wood
2010-03-25 16:59                     ` Grant Likely
2010-03-25 16:59                       ` Grant Likely
2010-03-25 17:03                       ` Timur Tabi
2010-03-25 17:35                         ` Grant Likely
2010-03-25 17:35                           ` Grant Likely
2010-03-25 18:05                           ` Timur Tabi
2010-03-25 19:53                           ` Scott Wood [this message]
2010-03-25 19:53                             ` Scott Wood
2010-03-25 20:04                             ` Timur Tabi
2010-03-25 21:54                               ` Grant Likely
2010-03-25 21:54                                 ` Grant Likely
2010-03-25 22:19                                 ` Timur Tabi
2010-03-25 22:19                                   ` Timur Tabi
2010-03-25 21:39                             ` Grant Likely
2010-03-25 21:39                               ` Grant Likely
2010-03-25 22:47                               ` Scott Wood
2010-03-25 22:47                                 ` Scott Wood
2010-03-25 21:22                       ` David Gibson
2010-03-25 21:22                         ` David Gibson
2010-03-26  1:26                     ` Grant Likely
2010-03-26  1:26                       ` Grant Likely
2010-03-26 15:17                       ` Timur Tabi
2010-03-26 15:17                         ` Timur Tabi
2010-03-26 18:20                         ` Grant Likely
2010-03-26 18:20                           ` Grant Likely
2010-03-26 18:39                           ` Timur Tabi
2010-03-26 18:44                             ` Grant Likely
2010-03-26 18:44                               ` Grant Likely
2010-03-26 18:48                               ` Timur Tabi
2010-03-26 18:48                                 ` Timur Tabi
2010-03-26 18:56                                 ` Grant Likely
2010-03-26 18:56                                   ` Grant Likely
2010-03-26 18:58                                 ` Mitch Bradley
2010-03-26 18:58                                   ` Mitch Bradley
2010-03-26 19:07                                   ` Grant Likely
2010-03-26 19:07                                     ` Grant Likely
2010-03-26 18:48                             ` Mitch Bradley
2010-03-26 18:48                               ` Mitch Bradley
2010-03-24 18:27         ` Scott Wood
2010-03-24 18:27           ` Scott Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BABBF54.3010104@freescale.com \
    --to=scottwood@freescale.com \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=grant.likely@secretlab.ca \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=timur@freescale.com \
    --cc=wmb@firmworks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.