Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/22/15 11:05, Rafał Miłecki wrote:
On 22 May 2015 at 10:31, Arend van Spriel<arend@xxxxxxxxxxxx>  wrote:
On 05/20/15 17:02, Rafał Miłecki wrote:

On 20 May 2015 at 14:09, Arend van Spriel<arend@xxxxxxxxxxxx>   wrote:

@@ -139,11 +165,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp)
          char *ekv;
          u32 cplen;

-       c = nvp->fwnv->data[nvp->pos];
-       if (!is_nvram_char(c)) {
+       c = nvp->data[nvp->pos];
+       if (!is_nvram_char(c)&&   (c != ' ')) {


Don't smuggle behavior changes in patches doing something else!


The subject is "Add support for host platform NVRAM loading" and guess what.
That type of NVRAM turned out to have spaces in the entries so in my opinion
it is related to this patch. I can split it up if you feel strongly about
this.

I'd expect such patch to just implement *loading* from different
source and nothing else. If there are additional changes needed, I
think they should go in separated patch if possible.

I noticed the same problem with parsing NVRAM values and sent
[PATCH] brcmfmac: allow NVRAM values to contain space and '#' chars
, so you should be able to drop this patch of your patch anyway.
You may give me an Ack if you have a moment :)

Whoops. I did not :-p I don't want to deal with '#' in value field as it is either invalid or irrelevant to firmware on the device.

Regards,
Arend

@@ -406,19 +434,34 @@ static void brcmf_fw_request_nvram_done(const
struct firmware *fw, void *ctx)
          struct brcmf_fw *fwctx = ctx;
          u32 nvram_length = 0;
          void *nvram = NULL;
+       u8 *data = NULL;
+       size_t data_len;
+       bool raw_nvram;

          brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev));
-       if (!fw&&   !(fwctx->flags&   BRCMF_FW_REQ_NV_OPTIONAL))
-               goto fail;
+       if ((fw)&&   (fw->data)) {


if (fw&&   fw->data)
will work just fine, I'm surprised checkpatch doesn't complain.

I ran checkpatch.pl --strict and did not get complaint about this change.

I know, it's weird. Maybe I'll report this an improvement idea to
checkpatch maintainer.







[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux