Messages in this thread | | | Date | Wed, 4 Jan 2023 10:32:36 +0100 | Subject | Re: [PATCH 1/2] brcmfmac: Use separate struct to declare firmware names for Apple OTP chips | From | Arend van Spriel <> |
| |
On 1/3/2023 2:46 PM, Hector Martin wrote: > On 03/01/2023 22.30, Arend van Spriel wrote: >> On 1/3/2023 4:55 AM, Hector Martin wrote: >>> On 2023/01/03 3:27, Arend Van Spriel wrote: >>>> On January 2, 2023 4:15:41 PM Hector Martin <marcan@marcan.st> wrote: >>>> >>>>> On 02/01/2023 23.40, Aditya Garg wrote: >>>>>> From: Aditya Garg <gargaditya08@live.com> >>>>>> >>>>>> Commit 'dce45ded7619' added support for 89459 chip pcie device. It uses the >>>>>> BRCM4355 chip which is also found in Apple hardware. However this commit >>>>>> causes conflicts in the firmware naming between Apple hardware, which >>>>>> supports OTP and other non-Apple hardwares. So, this patch makes these >>>>>> Apple chips use their own firmware table so as to avoid possible conflicts >>>>>> like these in the future. >>>>> >>>>> I think my reply to Arend flew over your head. >>>>> >>>>> My point was that I'd rather have the Broadcom/Cypress people actually >>>>> answer my question so we can figure out how to do this *properly*, >>>>> instead of doing "safer-but-dumb" things (like this patch) because we >>>>> just don't have the information to do it properly. >>>> >>>> Fair enough. Can you accurately (re)state your question and I will try to >>>> answer it. >>> >>> As per my original email: Is the CYW89459 just a rebrand of the BCM4355, >>> or just a subset? Can we consider them equivalent, and equivalent to the >>> Apple part (BCM4355C1 / revision 12)? >> >> There is probably no easy answer. Mainly because Cypress is a separate >> entity. However, they use the same/similar technology and code base. So >> let me first start with the chip naming. The wifi chip primarily has a >> number and revision. The chip number is straighforward and can be read >> from the device. The chip revision comes in two variants: 1) simple >> increasing number as read from the device, and 2) a <letter><digit> >> format. The latter start at a0, which you almost never see in the wild >> unless we do it "first time right". Whenever spinning a new chip we >> either increase the digit or the letter depending on type/amount of >> changes. There is not predictable mapping between the revision variants. >> Depending on the hurdles in a chip project we may move from a0 to b0, or >> from b0 to b1 or whatever. > > Right, this is standard chip spin numbering, that much I know. > >> If CYW89459 chip reads chip number 0x4355 than it is a BCM4355. If it is >> a different revision it may require different firmware. A different >> letter will always require different firmware. A different digit may >> work although the firmware can have code paths for a specific revision. > > So is it always correct to have the same firmware (in a generic > situation, not a customized OEM build) for, say, a BCM4355 rev 12, > regardless of what the PCI ID programmed into the OTP is (and what the > marketing device name is)?
Yes.
> If so, then my conclusion is that the original patch I replied to is > incorrect, all the defines should've been called BCM4355 (not the > Cypress part number), and we probably need two firmware table entries > since (judging by the revision check elsewhere in that patch) there are > other revisions in the wild than the one Apple uses, and therefore there > should at the very least be a firmware name split at C1. It would then > be very helpful to know what revisions *do* exist and their correct naming.
I can only track down what we have in Broadcom. For the 4355 the revisions B1 (=6), B3 (=8), C0 (=10) and C1 are mentioned as released. Here things get weird, because you mentioned BCM4355 rev12, which would be a C2. So without asking around I can only assume this C2 variant is not different from firmware perspective and can happily run the C1 firmware.
> If different PCI device IDs might need different firmware, then the > exiting firmware selection/table mechanism is insufficient. > >> Happy New year to you. Thanks for clearly marking the rant. Makes it >> easier to ignore, but let me get into this. I would not call bcmdhd the >> downstream driver. It is a separate out-of-tree driver. Indeed resources >> were pulled from brcm80211 development, but there always have been only >> 2 or 3 people working on it. Me being the constant working mule and >> these days only for 20% of my working hours to do the job. So you are >> not really doing our job as we are not assigned to do so. I guess there >> is no ROI for Broadcom or so it is perceived and there is no customer >> pushing for it. That said I am always happy to help and clarify whatever >> I can. > > Is there any chance you can provide a list of chips/shipping revisions > and revision IDs, so we can stop guessing at the mappings in the > firmware table? Because this is effectively breaking userspace ABI every > time we make a change to an existing chip, as it can change the firmware > file name that userspace loads. This already happened with BCM4364, > where (at least) B2 and B3 revisions exist in the wild and we need > separate firmwares, yet it was added with a full mask, resulting in > people copying "the right firmware for them" manually and my patch to > split it into properly named firmwares will break those users.
Userspace is not loading anything these days. AFAIK the kernel is directly accessing the firmware file. Anyway, I never considered this as being a big issue. If people change their installed os to get things working, they can expect the reverse can happen anytime and deal with it once more. If this is considered a real issue we should only set the revmask for the revision we know to be working.
Regards, Arend [unhandled content-type:application/pkcs7-signature] | |