2013-04-05 08:19:22

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: Version number policy!

On Thu, Apr 4, 2013 at 11:27 AM, Adrian Chadd <[email protected]> wrote:
> Hi,
>
> Here's my first take on the version number policy:
>
> https://github.com/qca/open-ath9k-htc-firmware/wiki/VersionPolicy
>
> The summary:
>
> * major version number changes are for firmware API / behaviour
> changes that aren't backwards compatible;
> * minor version number changes are for firmware API / behaviour
> changes that are incremental and default to the old behaviour (eg, new
> optional commands);
> * the driver should check the minor version number before using any
> optional features with that version.
>
> What's TODO:
>
> * Add a new WMI command to get the build number, git string, etc.
> * add it as our first optional minor version command :-)

This is better than anything we had drafted before for 802.11 open
firmware design rules. Cc'ing a few lists for wider review given that
what we had written before for rules was for 802.11 and Bluetooth [0]
and it was very Linux specific. We are striving for open firmware here
for the community, for BSD / Linux. Christian would have dealt with
more of the support on open firmware design so far due to carl9170.fw
[1] so curious if he has any input.

As for 802.11 the rules we defined were pretty silly about firmware
file names but should be considered. Here's the key thing I think we
should keep from those old guidelines:

"In general if you are just providing bug fixes you do not need to
provide a new firmware filename for the module, using the old filename
is fine so long as the same API was kept."

The rest I think your specification details better what we should do
in terms of API changes for versioning.

[0] http://wireless.kernel.org/en/developers/Documentation/firmware-versioning
[1] http://wireless.kernel.org/en/users/Drivers/carl9170.fw

Luis


2013-04-05 18:08:21

by Christian Lamparter

[permalink] [raw]
Subject: Re: Version number policy!

On Friday 05 April 2013 19:23:54 Eugene Krasnikov wrote:
> When it comes to feature bitmap do you have an example of such a
> bitmap from carl9170?
That's a weird way of asking. No, I don't have an example?!
But carl9170fw_feature_list is used in "production".

Anyway, there is one defined in:
<https://github.com/chunkeey/carl9170fw/blob/master/include/shared/fwdesc.h#L26>

It's packed into the OTUS firmware descriptor:
<https://github.com/chunkeey/carl9170fw/blob/master/include/shared/fwdesc.h#L118>

And the descriptor is filled in:
<https://github.com/chunkeey/carl9170fw/blob/master/carlfw/src/fw.c>

[Basically, a bit for every feature in kconfig.]

Of course, the nice thing is, if you have something like
kconfig (carl9170fw.git uses a pretty much the vanilla kernel
kconfig. Everything apart from the cmake scripts for gperf
(which is in extra/) is packed into the conf/ directory.

And it has the necessary cmake files. So it should integrate
well into the "new" ath9k_htc cmake build system Felix made.

> Why not to rely always on major version?
Ok, let's do the same with major version...

if ((fw_major == 1 && fw_minor > 30) ||
(fw_major == 2 && fw_minor > 7) ||
(fw_major == 3 && fw_minor > 3) ||
(fw_major > 4))
feature_supported = true;

Nope, I think it looks just as bad.
[Please, explain what you meant to say...
Because the new versioning draft stats that very
major release is incompatible with the old one and
clearly, having to support a ton of different firmware
trees is no joy.... and not necessary]

BTW: Instead of a GET_BUILD command, why not make a add a
firmware header or pack the information into the tail...

This way, it can be read and parsed by tools, without having
to upload it to the device first.

carl9170fw already such a tool, it's called: fwinfo and it is located in:
<https://github.com/chunkeey/carl9170fw/blob/master/tools/src/fwinfo.c>

Regards,
Christian

2013-04-08 20:37:13

by Eugene Krasnikov

[permalink] [raw]
Subject: Re: Version number policy!

Christian,

Did you have such a situation when e.g. one feature has 2 incompatible
implementations? Let?s say initial implementation of ?offloaded TX
rate control? feature was immature and community decides to change
implementation drastically so final realization of this feature is
incompatible with initial. How did carl9170 track such a situation in
feature bitmap?

2013/4/8 Adrian Chadd <[email protected]>:
> On 8 April 2013 08:33, Christian Lamparter <[email protected]> wrote:
>> On Monday, April 08, 2013 11:10:30 AM Eugene Krasnikov wrote:
>>> It?s a good idea to pack bitmap into the tail/header of the firmware
>>> binary to get capabilities even before fw loading. The plan is to add
>>> 8 bytes for caps and also add time stamp. Let me play around with that
>>> and provide fw and driver patch for review.
>>
>> Hm, come to think of it. Kalle, do you think ath9k_htc could use
>> some bits of the fw header format, parser or the complete infrastructure
>> from ath6kl? This could be great for Adrian, because he could
>> just point people to it and they could moved it into the code
>> into /drivers/net/wireless/ath/fw.c.
>
> Just remember that we (ath9k_htc and carl9170) aren't constrained by
> the same kinds of issues that closed firmware is. So version checks
> aren't that bad, because that way over time we don't end up needing to
> maintain lots of special cases for firmware options.
>
> I still like the idea of firmware options for build time options that
> people may wish to use - eg, say we want to support a version of
> ath9k-htc firmware that does offloaded TX rate control, versus not.
> But, maintaining multiple builds of the same firmware is IMHO going to
> be a path towards madness to maintain.
>
> The maintainability is why I'm hoping (!) to keep the number of
> options low and just do "clean slate" moves whenever we shift to the
> next major firmware version.
>
>
> Adrian



--
Best regards,
Eugene

2013-04-05 17:23:54

by Eugene Krasnikov

[permalink] [raw]
Subject: Re: Version number policy!

Good point regarding timestamp.

When it comes to feature bitmap do you have an example of such a
bitmap from carl9170? Why not to rely always on major version?

2013/4/5 Christian Lamparter <[email protected]>:
> On Friday 05 April 2013 10:19:00 Luis R. Rodriguez wrote:
>> On Thu, Apr 4, 2013 at 11:27 AM, Adrian Chadd <[email protected]> wrote:
>> > Here's my first take on the version number policy:
>> >
>> > https://github.com/qca/open-ath9k-htc-firmware/wiki/VersionPolicy
>> > The summary:
>> >
>> > * major version number changes are for firmware API / behaviour
>> > changes that aren't backwards compatible;
>> > * minor version number changes are for firmware API / behaviour
>> > changes that are incremental and default to the old behaviour (eg, new
>> > optional commands);
>> > * the driver should check the minor version number before using any
>> > optional features with that version.
>> >
>> > What's TODO:
>> >
>> > * Add a new WMI command to get the build number, git string, etc.
>> > * add it as our first optional minor version command :-)
>>
>> This is better than anything we had drafted before for 802.11 open
>> firmware design rules. Cc'ing a few lists for wider review given that
>> what we had written before for rules was for 802.11 and Bluetooth [0]
>> and it was very Linux specific. We are striving for open firmware here
>> for the community, for BSD / Linux. Christian would have dealt with
>> more of the support on open firmware design so far due to carl9170.fw
>> [1] so curious if he has any input.
> Based on my experience with carl9170, I can tell you that
> new stuff (new wmi commands, or advanced offload caps, features
> and bugfixes) should be advised via feature flags in bitmaps
> and not firmware versions. [Just make it long enough...]
>
> Otherwise you'll have to write endless checks like:
> if ((fw_minor == 1 && fw_patch > 30) ||
> (fw_minor == 2 && fw_patch > 7) ||
> (fw_minor == 3 && fw_patch > 3) ||
> (fw_minor > 4))
> feature_supported = true;
>
> everytime you backport features and bugfixes to older firmwares.
>
> Also, firmware dates are more important than you think.
> They allow some way of syncing the firmware->driver and
> your inbox.
>
> Regards,
> Christian
>
> _______________________________________________
> Ath9k_htc_fw mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/ath9k_htc_fw



--
Best regards,
Eugene

2013-04-06 05:52:23

by Kalle Valo

[permalink] [raw]
Subject: Re: Version number policy!

Eugene Krasnikov <[email protected]> writes:

> Good point regarding timestamp.
>
> When it comes to feature bitmap do you have an example of such a
> bitmap from carl9170? Why not to rely always on major version?

I'm with Christian here. In ath6kl we switched to firmware advertising
feature capabilities and I have never looked back, it's so much easier.
You don't need to do any mapping between firmware versions and features,
you just deal one feature at a time.

--
Kalle Valo

2013-04-05 08:21:08

by Adrian Chadd

[permalink] [raw]
Subject: Re: Version number policy!

On 5 April 2013 01:19, Luis R. Rodriguez <[email protected]> wrote:

> This is better than anything we had drafted before for 802.11 open
> firmware design rules. Cc'ing a few lists for wider review given that
> what we had written before for rules was for 802.11 and Bluetooth [0]
> and it was very Linux specific. We are striving for open firmware here
> for the community, for BSD / Linux. Christian would have dealt with
> more of the support on open firmware design so far due to carl9170.fw
> [1] so curious if he has any input.
>

I'm treating it like shared libraries should be treated - minor
versions shouldn't break existing stuff, just add new calls. Major
versions are incompatible.

We'll see how well this scales after people hack at it for a while.



Adrian

2013-04-09 11:28:33

by Theodore Ts'o

[permalink] [raw]
Subject: Re: Version number policy!

On Mon, Apr 08, 2013 at 06:51:10PM -0700, Adrian Chadd wrote:
>
> But then you end up with people making filesystems which aren't
> necessarily backwards compatible (and aren't aware of this), then try
> to share with other extX implementations; or boot an older Linux
> kernel (eg plugging an ext3 device on a newer box to an older box.)
> Now, ext3 is a bit more mature nowdays so people aren't _always_
> hitting this corner case.

You have a long memory --- going back to about seven years ago, to
2006....

> I'm not knocking extX here; I'm just pointing out that exposing things
> as a set of capability flags doesn't magically fix interoperability.
> It just stops you from scribbling crap where it shouldn't be.

Sure, the only question is how long do you support ancient file
systems out there. I consider that primarily a distribution question,
not an upstream development question. People who pay $$$ for ancient
crud, expect that the people to whom they have paid $$$ will take care
of that problem for them. From an upstream development perspective,
some people might argue that two years is plenty of time to wait for
the vast majority of the people running community distributions to
have upgraded (and if they want more than that, they can pay $$$ to
the aforementioned enterprise distro's, or take care of that situation
on their own).

- Ted

2013-04-08 22:03:47

by Christian Lamparter

[permalink] [raw]
Subject: Re: Version number policy!

Hello,

On Monday, April 08, 2013 10:37:08 PM Eugene Krasnikov wrote:
> Did you have such a situation when e.g. one feature has 2 incompatible
> implementations?
I'm not aware of anything in particular. But we can look elsewhere for
examples, maybe Kalle knows one? Or you could ask the file system folks.
AFAIK ext2 fs is using one and they have maintained some basic
compatibility throughout the major releases.

For example, it's possible to read data on a ext3 and ext4(dev) file
system with the old ext2 module - this is very user-friendly [1]!
Maybe, you can dig up more examples, just browse the web for "feature
bits/flags/..."

> 2013/4/8 Adrian Chadd <[email protected]>:
> > On 8 April 2013 08:33, Christian Lamparter <[email protected]> wrote:
> >> On Monday, April 08, 2013 11:10:30 AM Eugene Krasnikov wrote:
> >>> It?s a good idea to pack bitmap into the tail/header of the firmware
> >>> binary to get capabilities even before fw loading. The plan is to add
> >>> 8 bytes for caps and also add time stamp. Let me play around with that
> >>> and provide fw and driver patch for review.
> >>
> >> Hm, come to think of it. Kalle, do you think ath9k_htc could use
> >> some bits of the fw header format, parser or the complete infrastructure
> >> from ath6kl? This could be great for Adrian, because he could
> >> just point people to it and they could moved it into the code
> >> into /drivers/net/wireless/ath/fw.c.
> >
> > Just remember that we (ath9k_htc and carl9170) aren't constrained by
> > the same kinds of issues that closed firmware is. So version checks
> > aren't that bad, because that way over time we don't end up needing to
> > maintain lots of special cases for firmware options.

I know, I know, but wouldn't it be great to have a common firmware parsing
infrastructure for all the driver with firmware under ath/? [No, I'm not
talking about writing a firmware that works on all devices. I want common
rules for firmware header descriptors. But if you peg features (firmware
or device!) to abstract firmware versions like "1.2.3.4-34+", then this is
going to be difficult.]

> > I still like the idea of firmware options for build time options that
> > people may wish to use - eg, say we want to support a version of
> > ath9k-htc firmware that does offloaded TX rate control, versus not.
> > But, maintaining multiple builds of the same firmware is IMHO going to
> > be a path towards madness to maintain.
> >
> > The maintainability is why I'm hoping (!) to keep the number of
> > options low and just do "clean slate" moves whenever we shift to the
> > next major firmware version.

Or like ext2,3,4 => just have a lowest common denominator. So users can
at least always access the web and get a matching firmware.

Of course, it could be that we are just talking about completely different
things. Anyway, Luis asked me, if I have any "input"... and not more ;-).

Regards,
Christian

[1]
<https://ext4.wiki.kernel.org/index.php/Frequently_Asked_Questions#Can_I_mount_existing_Ext3_as_Ext4.3F_And_vice_versa.3F_Similarly_from_Ext2_to_Ext4_and_its_reverse.3F>

2013-04-09 00:37:39

by Theodore Ts'o

[permalink] [raw]
Subject: Re: Version number policy!

On Mon, Apr 08, 2013 at 04:12:07PM -0700, Adrian Chadd wrote:
> I'm a FreeBSD user. I know what it's like to have an alternate, out of
> linux implementation of ext2/3 that doesn't implement all of the
> features.
>
> I also know what kinds of crap happens when you try mounting a more
> recent extX filesystem on an earlier kernel. Ie, it doesn't
> necessarily end well. :-)

There shouldn't be any crap; just a an error message indicating that
"the file system has features which this implementation doesn't
understand". At least, if the implementation was competently
coded.... (ext2/3/4 has feature bitmasks that make it very clear what
features are required so that an implementation can mount the file
system read/write or read/only).

- Ted

2013-04-09 01:51:12

by Adrian Chadd

[permalink] [raw]
Subject: Re: Version number policy!

On 8 April 2013 17:37, Theodore Ts'o <[email protected]> wrote:

> There shouldn't be any crap; just a an error message indicating that
> "the file system has features which this implementation doesn't
> understand". At least, if the implementation was competently
> coded.... (ext2/3/4 has feature bitmasks that make it very clear what
> features are required so that an implementation can mount the file
> system read/write or read/only).

Right, the design side of it is fine.

But then you end up with people making filesystems which aren't
necessarily backwards compatible (and aren't aware of this), then try
to share with other extX implementations; or boot an older Linux
kernel (eg plugging an ext3 device on a newer box to an older box.)
Now, ext3 is a bit more mature nowdays so people aren't _always_
hitting this corner case. But I do recall earlier on when things were
moving forward quite quickly, people would create drives on Linux
machine X that couldn't be read or written to on Linux machine Y.

I'm not knocking extX here; I'm just pointing out that exposing things
as a set of capability flags doesn't magically fix interoperability.
It just stops you from scribbling crap where it shouldn't be.



Adrian

2013-04-08 19:00:13

by Adrian Chadd

[permalink] [raw]
Subject: Re: Version number policy!

On 8 April 2013 08:33, Christian Lamparter <[email protected]> wrote:
> On Monday, April 08, 2013 11:10:30 AM Eugene Krasnikov wrote:
>> It?s a good idea to pack bitmap into the tail/header of the firmware
>> binary to get capabilities even before fw loading. The plan is to add
>> 8 bytes for caps and also add time stamp. Let me play around with that
>> and provide fw and driver patch for review.
>
> Hm, come to think of it. Kalle, do you think ath9k_htc could use
> some bits of the fw header format, parser or the complete infrastructure
> from ath6kl? This could be great for Adrian, because he could
> just point people to it and they could moved it into the code
> into /drivers/net/wireless/ath/fw.c.

Just remember that we (ath9k_htc and carl9170) aren't constrained by
the same kinds of issues that closed firmware is. So version checks
aren't that bad, because that way over time we don't end up needing to
maintain lots of special cases for firmware options.

I still like the idea of firmware options for build time options that
people may wish to use - eg, say we want to support a version of
ath9k-htc firmware that does offloaded TX rate control, versus not.
But, maintaining multiple builds of the same firmware is IMHO going to
be a path towards madness to maintain.

The maintainability is why I'm hoping (!) to keep the number of
options low and just do "clean slate" moves whenever we shift to the
next major firmware version.


Adrian

2013-04-05 16:42:50

by Eugene Krasnikov

[permalink] [raw]
Subject: Re: Version number policy!

Hi Adrian,

This is the patch with new wmi command to support build number. Please
let me know if it's ok so i can send it as pull request:

>From 2591efa83bd24a807e3d93c4c8e1bf5c570737e1 Mon Sep 17 00:00:00 2001
From: Eugene Krasnikov <[email protected]>
Date: Fri, 5 Apr 2013 18:37:26 +0200
Subject: [PATCH] Add WMI_GET_BUILD_NUMBER

New command WMI_GET_BUILD_NUMBER has
following fields:
major
minor
tag

Signed-off-by: Eugene Krasnikov <[email protected]>
---
target_firmware/wlan/if_ath.c | 14 ++++++++++++++
target_firmware/wlan/include/k2/wlan_cfg.h | 5 ++++-
target_firmware/wlan/include/magpie/wlan_cfg.h | 3 +++
target_firmware/wlan/include/wlan_hdr.h | 6 ++++++
target_firmware/wlan/include/wmi.h | 1 +
5 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/target_firmware/wlan/if_ath.c b/target_firmware/wlan/if_ath.c
index 409aecf..034b8ea 100755
--- a/target_firmware/wlan/if_ath.c
+++ b/target_firmware/wlan/if_ath.c
@@ -1729,6 +1729,19 @@ static void ath_rc_mask_tgt(void *Context,
A_UINT16 Command,
wmi_cmd_rsp(sc->tgt_wmi_handle, Command, SeqNo, NULL, 0);
}

+static void ath_get_build_number_tgt(void *Context,A_UINT16 Command,
A_UINT16 SeqNo,
+ A_UINT8 *data, a_int32_t datalen)
+{
+ struct ath_softc_tgt *sc = (struct ath_softc_tgt *)Context;
+ struct wmi_build_number bld_num;
+
+ bld_num.major = ATH_BUILD_NUMBER_MAJOR;
+ bld_num.minor = ATH_BUILD_NUMBER_MINOR;
+ bld_num.tag = ATH_BUILD_NUMBER_TAG;
+
+ wmi_cmd_rsp(sc->tgt_wmi_handle, Command, SeqNo, &bld_num, sizeof(bld_num));
+}
+
static WMI_DISPATCH_ENTRY Magpie_Sys_DispatchEntries[] =
{
{handle_echo_command, WMI_ECHO_CMDID, 0},
@@ -1762,6 +1775,7 @@ static WMI_DISPATCH_ENTRY Magpie_Sys_DispatchEntries[] =
{ath_tx_stats_tgt, WMI_TX_STATS_CMDID, 0},
{ath_rx_stats_tgt, WMI_RX_STATS_CMDID, 0},
{ath_rc_mask_tgt, WMI_BITRATE_MASK_CMDID, 0},
+ {ath_get_build_number_tgt, WMI_GET_BUILD_NUMBER, 0}
};

/*****************/
diff --git a/target_firmware/wlan/include/k2/wlan_cfg.h
b/target_firmware/wlan/include/k2/wlan_cfg.h
index ce42d2c..5ad7d0a 100755
--- a/target_firmware/wlan/include/k2/wlan_cfg.h
+++ b/target_firmware/wlan/include/k2/wlan_cfg.h
@@ -82,7 +82,10 @@

#define ATH_VERSION_MAJOR 1
#define ATH_VERSION_MINOR 4
-
+
+#define ATH_BUILD_NUMBER_MAJOR 1
+#define ATH_BUILD_NUMBER_MINOR 1
+#define ATH_BUILD_NUMBER_TAG 1
/************************** HAL configurations **************************/

#define HAL_DESC_OPTIMIZATION
diff --git a/target_firmware/wlan/include/magpie/wlan_cfg.h
b/target_firmware/wlan/include/magpie/wlan_cfg.h
index 2590e9f..c66bf06 100755
--- a/target_firmware/wlan/include/magpie/wlan_cfg.h
+++ b/target_firmware/wlan/include/magpie/wlan_cfg.h
@@ -68,6 +68,9 @@
#define ATH_VERSION_MAJOR 1
#define ATH_VERSION_MINOR 4

+#define ATH_BUILD_NUMBER_MAJOR 1
+#define ATH_BUILD_NUMBER_MINOR 1
+#define ATH_BUILD_NUMBER_TAG 1
/************************** HAL configurations **************************/

#define HAL_DESC_OPTIMIZATION
diff --git a/target_firmware/wlan/include/wlan_hdr.h
b/target_firmware/wlan/include/wlan_hdr.h
index cfcfa0a..838f2bf 100755
--- a/target_firmware/wlan/include/wlan_hdr.h
+++ b/target_firmware/wlan/include/wlan_hdr.h
@@ -221,6 +221,12 @@ struct ath_aggr_info {
a_uint8_t padding;
};

+struct wmi_build_number {
+ a_uint16_t major;
+ a_uint16_t minor;
+ a_uint16_t tag;
+};
+
struct wmi_data_delba {
a_uint8_t ni_nodeindex;
a_uint8_t tidno;
diff --git a/target_firmware/wlan/include/wmi.h
b/target_firmware/wlan/include/wmi.h
index 47bf4a7..9c577a6 100755
--- a/target_firmware/wlan/include/wmi.h
+++ b/target_firmware/wlan/include/wmi.h
@@ -162,6 +162,7 @@ typedef enum {
WMI_TX_STATS_CMDID,
WMI_RX_STATS_CMDID,
WMI_BITRATE_MASK_CMDID,
+ WMI_GET_BUILD_NUMBER
} WMI_COMMAND_ID;

/*
--
1.7.9.5

2013/4/5 Adrian Chadd <[email protected]>:
> On 5 April 2013 01:19, Luis R. Rodriguez <[email protected]> wrote:
>
>> This is better than anything we had drafted before for 802.11 open
>> firmware design rules. Cc'ing a few lists for wider review given that
>> what we had written before for rules was for 802.11 and Bluetooth [0]
>> and it was very Linux specific. We are striving for open firmware here
>> for the community, for BSD / Linux. Christian would have dealt with
>> more of the support on open firmware design so far due to carl9170.fw
>> [1] so curious if he has any input.
>>
>
> I'm treating it like shared libraries should be treated - minor
> versions shouldn't break existing stuff, just add new calls. Major
> versions are incompatible.
>
> We'll see how well this scales after people hack at it for a while.
>
>
>
> Adrian
>
> _______________________________________________
> Ath9k_htc_fw mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/ath9k_htc_fw



--
Best regards,
Eugene

2013-04-08 15:33:16

by Christian Lamparter

[permalink] [raw]
Subject: Re: Version number policy!

On Monday, April 08, 2013 11:10:30 AM Eugene Krasnikov wrote:
> It?s a good idea to pack bitmap into the tail/header of the firmware
> binary to get capabilities even before fw loading. The plan is to add
> 8 bytes for caps and also add time stamp. Let me play around with that
> and provide fw and driver patch for review.

Hm, come to think of it. Kalle, do you think ath9k_htc could use
some bits of the fw header format, parser or the complete infrastructure
from ath6kl? This could be great for Adrian, because he could
just point people to it and they could moved it into the code
into /drivers/net/wireless/ath/fw.c.

Regards,
Christian

2013-04-08 23:12:09

by Adrian Chadd

[permalink] [raw]
Subject: Re: Version number policy!

I'm a FreeBSD user. I know what it's like to have an alternate, out of
linux implementation of ext2/3 that doesn't implement all of the
features.

I also know what kinds of crap happens when you try mounting a more
recent extX filesystem on an earlier kernel. Ie, it doesn't
necessarily end well. :-)

Anyway, there's been a lot of good points in this discussion. Let's
keep it in mind as we slowly march forward.

There's only like, 60k of generated binary code on these things. Let's
not overthink things too much for now. :)



Adrian

2013-04-08 09:10:31

by Eugene Krasnikov

[permalink] [raw]
Subject: Re: Version number policy!

Thanx Christian for a valuable input.

It?s a good idea to pack bitmap into the tail/header of the firmware
binary to get capabilities even before fw loading. The plan is to add
8 bytes for caps and also add time stamp. Let me play around with that
and provide fw and driver patch for review.

2013/4/6 Adrian Chadd <[email protected]>:
> On 5 April 2013 22:52, Kalle Valo <[email protected]> wrote:
>> Eugene Krasnikov <[email protected]> writes:
>>
>>> Good point regarding timestamp.
>>>
>>> When it comes to feature bitmap do you have an example of such a
>>> bitmap from carl9170? Why not to rely always on major version?
>>
>> I'm with Christian here. In ath6kl we switched to firmware advertising
>> feature capabilities and I have never looked back, it's so much easier.
>> You don't need to do any mapping between firmware versions and features,
>> you just deal one feature at a time.
>
> Ok. Well, we've got plenty of time to finalise this before things get
> established. There's still plenty of tidying up to do before we start
> pushing in new features.
>
> Thanks for all your feedback so far!
>
>
>
> Adrian



--
Best regards,
Eugene

2013-04-06 07:08:36

by Adrian Chadd

[permalink] [raw]
Subject: Re: Version number policy!

On 5 April 2013 22:52, Kalle Valo <[email protected]> wrote:
> Eugene Krasnikov <[email protected]> writes:
>
>> Good point regarding timestamp.
>>
>> When it comes to feature bitmap do you have an example of such a
>> bitmap from carl9170? Why not to rely always on major version?
>
> I'm with Christian here. In ath6kl we switched to firmware advertising
> feature capabilities and I have never looked back, it's so much easier.
> You don't need to do any mapping between firmware versions and features,
> you just deal one feature at a time.

Ok. Well, we've got plenty of time to finalise this before things get
established. There's still plenty of tidying up to do before we start
pushing in new features.

Thanks for all your feedback so far!



Adrian

2013-04-05 19:16:48

by Adrian Chadd

[permalink] [raw]
Subject: Re: Version number policy!

The point here is that once we hit a new major version, all the
previous version checks can go away.

So, if we decide to change some WMI message formats, it'll be in 2.x
rather than 1.x.

For example, has talked about killing off rate control in the NIC and
doing it on the host. I'm happy with that (since it's easy enough to
move back into the NIC if people want it for whatever reason) so:

* Do we make it configurable?
* Do we make it backwards compatible?
* Do we just say "oh screw this" and treat it as part of the next
major revision?

If I were doing this for work and there was a long lifetime
requirement for this stuff, then sure, we may have pressure from
various groups to maintain backwards compatibility. And as we do so,
we end up slowly bloating out the firmware and adding more driver
capabilities.

But since we're doing this in an open community, we can just create
branches and let people who wish to maintain things do so, whilst not
tying ourselves down too much. We have very specific pressures here
(ie, how much RAM and CPU these little USB SoCs have), so we can't
just infinitely grow the API.

If there are bugfixes that someone like Google, Redhat or Canonical
need for the 1.3 firmware in say, 2015 (when we've moved onto a much
later evolution of this firmware) because they have a compat-wireless
download that _only gets bugfixes_ and doesn't get new features (ie
,doesn't just sync against the latest compat-wireless to get bugfixes
_and_ features), then they're quite welcome to cherry-pick fixes from
mainline, merge them into the 1.3 branch and send a diff or pull
request. But I personally don't want to maintain config options and
backwards compatibility outside of what goes on in a minor branch.

So, to address Christian's concern in more specific detail:

* I intend that major revisions are break points, and that when we
push out a new major revision, ath9k_htc (or whatever it becomes) will
eventually update to use that;
* .. then we can do things that break backwards compatibility, which
means we can kill all those capability checks if we need to;
* For things that occur during a given major release branch, we can
either use a capability bitmap OR we can do it via versioning -
because I don't want to see ath9k_htc try to support >1 major version;
* How we do that check isn't really that big a deal - as long as the
checks look like:

if (feature_xxx(sc)) {
}

.. we can easily just change feature_xxx() to be a bitmap check or a
version check.

* And then once we push out a new major release, we can reap features
that are either on by default in the new major release.

How's that sound?


Adrian

2013-04-05 17:03:17

by Christian Lamparter

[permalink] [raw]
Subject: Re: Version number policy!

On Friday 05 April 2013 10:19:00 Luis R. Rodriguez wrote:
> On Thu, Apr 4, 2013 at 11:27 AM, Adrian Chadd <[email protected]> wrote:
> > Here's my first take on the version number policy:
> >
> > https://github.com/qca/open-ath9k-htc-firmware/wiki/VersionPolicy
> > The summary:
> >
> > * major version number changes are for firmware API / behaviour
> > changes that aren't backwards compatible;
> > * minor version number changes are for firmware API / behaviour
> > changes that are incremental and default to the old behaviour (eg, new
> > optional commands);
> > * the driver should check the minor version number before using any
> > optional features with that version.
> >
> > What's TODO:
> >
> > * Add a new WMI command to get the build number, git string, etc.
> > * add it as our first optional minor version command :-)
>
> This is better than anything we had drafted before for 802.11 open
> firmware design rules. Cc'ing a few lists for wider review given that
> what we had written before for rules was for 802.11 and Bluetooth [0]
> and it was very Linux specific. We are striving for open firmware here
> for the community, for BSD / Linux. Christian would have dealt with
> more of the support on open firmware design so far due to carl9170.fw
> [1] so curious if he has any input.
Based on my experience with carl9170, I can tell you that
new stuff (new wmi commands, or advanced offload caps, features
and bugfixes) should be advised via feature flags in bitmaps
and not firmware versions. [Just make it long enough...]

Otherwise you'll have to write endless checks like:
if ((fw_minor == 1 && fw_patch > 30) ||
(fw_minor == 2 && fw_patch > 7) ||
(fw_minor == 3 && fw_patch > 3) ||
(fw_minor > 4))
feature_supported = true;

everytime you backport features and bugfixes to older firmwares.

Also, firmware dates are more important than you think.
They allow some way of syncing the firmware->driver and
your inbox.

Regards,
Christian