public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Date: Fri, 01 Jul 2016 11:20:26 +0200	[thread overview]
Message-ID: <4973367.zalLWgQG3c@wuerfel> (raw)
In-Reply-To: <b18cd232-711a-b617-2f09-09c4bdf2c3c4@broadcom.com>

On Friday, July 1, 2016 10:17:37 AM CEST Arend Van Spriel wrote:
> 
> On 1-7-2016 4:08, Rob Herring wrote:
> > On Wed, Jun 29, 2016 at 04:04:31PM +0200, Hans de Goede wrote:
> >> Add a brcm,nvram_file_name dt property to allow overruling the default
> >> nvram filename for sdio devices. The idea is that we can specify a
> >> board specific nvram file, e.g. brcmfmac43362-ap6210.txt for boards
> >> with an ap6210 wifi sdio module and ship this in linux-firmware, so
> >> that wifi will work out of the box, without requiring users to find
> >> and then manually install the right nvram file for their board.
> > 
> > What about putting its contents directly into DT? It's just text 
> > key/value pairs so it would match up well.
> 
> It would be an option, but I have no clue how to dig up documentation of
> these key,value pairs. The file is typically obtained from a wifi module
> vendor which would need to be converted to DT format, ie. prefix with
> "brcm," etc. From driver perspective this would mean it need to know
> keys. Currently, it just takes the file contents and sends it to the device.

I can see multiple ways to do this here:

- create a child node for the nvram settings and document that the properties
  in there follow exactly the format that is used for the nvram file.

- have just one property that contains a copy of the entire nvram file,
  serving as a backing store for the file instead of pointing to the file.

- figure out more about the actual properties that are required and then
  document only the ones that are actually different between devices using
  the same chip. With a per-chip file that we can put into linux-firmware,
  and the data from the OTP ROM, we might need only a small subset here
  that fits well enough in the DT.

> >> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> >> index 5dbf169..2ba13a6 100644
> >> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> >> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> >> @@ -11,6 +11,7 @@ Required properties:
> >>  Optional properties:
> >>   - brcm,drive-strength : drive strength used for SDIO pins on device in mA
> >>  	(default = 6).
> >> + - brcm,nvram_file_name : name of the nvram file to load
> > 
> > The need for firmware file names has come up several times though 
> > nothing merged to yet. There has been at least some level of agreement 
> > to use "firmware-name" here. 
> 
> Do you mean with or without vendor prefix? Actually the device needs two
> files to initialize. The firmware that is needed to have anything
> running on the device and the nvram data that characterizes the device
> for that firmware.

I thought the outcome was to never put file names into DT for new bindings, because
it doesn't do what you want in the end: Either the "compatible" string correctly
identifies the device and can be used to construct or look up a file name, or
the compatible string is not enough to actually identify the device and should
be extended with a more specific one.

Having the module identifer in a property to use that along with the compatible
string for the file name would also work, but I think just using the compatible
string list by itself makes more sense.

	Arnd

  reply	other threads:[~2016-07-01  9:20 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-29 14:04 [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property Hans de Goede
2016-06-29 14:04 ` [PATCH 2/4] ARM: dts: sun7i-a20-cubietruck: Set brcm,nvram_file_name Hans de Goede
2016-06-29 17:01   ` [PATCH 2/4] ARM: dts: sun7i-a20-cubietruck: Set brcm, nvram_file_name Kalle Valo
2016-06-29 18:01     ` [linux-sunxi] Re: [PATCH 2/4] ARM: dts: sun7i-a20-cubietruck: Set brcm,nvram_file_name Hans de Goede
2016-06-29 14:04 ` [PATCH 3/4] ARM: dts: sun7i-a20-wits-pro-a20-dkt: Set brcm, nvram_file_name Hans de Goede
2016-06-29 14:04 ` [PATCH 4/4] ARM: dts: sun5i-a10s-auxtek-t004: " Hans de Goede
2016-06-29 14:42 ` [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property Jonas Gorski
2016-06-29 15:16   ` Hans de Goede
2016-06-29 17:00     ` Kalle Valo
2016-06-29 18:01       ` [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm, nvram_file_name " Hans de Goede
2016-06-29 18:51         ` Arend Van Spriel
2016-06-29 18:57           ` Arend Van Spriel
2016-06-30  8:50             ` Kalle Valo
2016-06-29 19:33           ` Arnd Bergmann
2016-06-29 19:54             ` [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name " Priit Laes
2016-06-29 20:07               ` [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm, nvram_file_name " Arnd Bergmann
2016-06-30  9:02               ` Kalle Valo
2016-06-30  9:50                 ` Hans de Goede
2016-06-30  9:58                   ` Kalle Valo
2016-06-30 10:04                     ` Hans de Goede
2016-06-30 10:18                       ` Jonas Gorski
2016-06-30 10:25                         ` Hans de Goede
2016-06-30 11:31                           ` Arnd Bergmann
2016-06-30 19:23                             ` Arend Van Spriel
2016-07-01  8:51                               ` Arnd Bergmann
2016-07-01  8:58                               ` Jonas Gorski
2016-07-02  6:59                                 ` Kalle Valo
2016-07-02 18:20                                 ` Arend Van Spriel
2016-07-02 21:30                                   ` Arnd Bergmann
2016-07-04  8:41                                     ` Arend Van Spriel
2016-07-04  8:55                                       ` Arnd Bergmann
2016-07-04  9:08                                         ` Arend Van Spriel
2016-07-04 14:54                                           ` Arnd Bergmann
2016-07-04 18:36                                             ` Arend van Spriel
2016-07-05 13:43                                               ` Arnd Bergmann
2016-07-06  8:08                                                 ` Arend Van Spriel
2016-07-06 13:42                                                   ` Arnd Bergmann
2016-07-06 19:19                                                     ` Arend Van Spriel
2016-07-07  8:46                                                       ` Arnd Bergmann
2016-07-07  9:16                                                         ` Arend Van Spriel
2016-07-07  9:24                                                           ` Arnd Bergmann
2016-07-17 21:45                                                         ` Rob Herring
2016-07-18  7:51                                                           ` Arend Van Spriel
2016-06-30  8:46           ` Kalle Valo
2016-06-30  9:49             ` Hans de Goede
2016-06-30  9:53           ` Hans de Goede
2016-07-01  2:08 ` [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name " Rob Herring
2016-07-01  8:17   ` Arend Van Spriel
2016-07-01  9:20     ` Arnd Bergmann [this message]
2016-07-04 16:12     ` Rob Herring

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=4973367.zalLWgQG3c@wuerfel \
    --to=arnd@arndb.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox