Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190602102144.17360-1-chunkeey@gmail.com>
In-Reply-To: <20190602102144.17360-1-chunkeey@gmail.com>
From:   Julian Calaby <julian.calaby@gmail.com>
Date:   Sun, 2 Jun 2019 21:43:52 +1000
Message-ID: <CAGRGNgXqB=5oi1Nq4ZNSk3csOEr4A6WgN8QymKMriTcevnKUQw@mail.gmail.com>
Subject: Re: [RFC PATCH] ath9k: integrate AR92XX pci fixup code
To:     Christian Lamparter <chunkeey@gmail.com>
Cc:     QCA ath9k Development <ath9k-devel@qca.qualcomm.com>,
        linux-wireless@vger.kernel.org, Hauke Mehrtens <hauke@hauke-m.de>,
        Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Hi Christian,

On Sun, Jun 2, 2019 at 8:24 PM Christian Lamparter <chunkeey@gmail.com> wrote:
>
> Some devices (like the Cisco Meraki Z1 Cloud Managed Teleworker Gateway)
> need to be able to initialize the PCIe wifi device. Normally, this is done
> during the early stages of booting linux, because the necessary init code
> is read from the memory mapped SPI and passed to pci_enable_ath9k_fixup.
> However, this isn't possible for devices which have the init code for the
> Atheros chip stored on NAND in an UBI volume. Hence, this module can be
> used to initialize the chip when the user-space is ready to extract the
> init code.
>
> Martin Blumenstingl prodived the following fixes:
> owl-loader: add support for OWL emulation PCI devices
> owl-loader: don't re-scan the bus when ath9k_pci_fixup failed
> owl-loader: use dev_* instead of pr_* logging functions
> owl-loader: auto-generate the eeprom filename as fallback
> owl-loader: add a debug message when swapping the eeprom data
> owl-loader: add missing newlines in log messages
>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Two questions:

1. This seems complicated enough that the functions introduced should
probably go into a separate .c file, maybe "noeeprom.c", with a header
file with all the ifdef / config magic in it.
2. This smells almost like a completely separate PCI(e) driver for
cards in a "weird" state. Is there anything you're using from ath9k
other than the eeprom file naming? and is that really useful? Won't
the eeprom files be device specific and therefore could always use the
device name fallback?

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/