Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On 17-7-2016 23:45, Rob Herring wrote: > On Thu, Jul 07, 2016 at 10:46:28AM +0200, Arnd Bergmann wrote: >> On Wednesday, July 6, 2016 9:19:41 PM CEST Arend Van Spriel wrote: >>> On 6-7-2016 15:42, Arnd Bergmann wrote: On Wednesday, July 6, 2016 10:08:55 AM CEST Arend Van Spriel wrote: > On Tue, Jul 5, 2016 at 3:43 PM, Arnd Bergmann wrote: All existing uses of the model property in arch/arm/boot/dts and most of the ones in arch/powerpc/boot/dts are against the intended usage in one way or another, but adding different kind of incorrect usage won't improve that. The only way I can see the model property being used correctly would be to have it match the first entry in the compatible property, but that is completely redundant, so we tend to omit it, except for the root node in which it is required. For the root node however, the historic practice that has crept in on ARM is to put something completely different in there, which is a human-readable description of the machine rather than something we can use as a unique indentifier. I'd just consider the "model" property burned, and not use it for anything that doesn't already use it, just like we handle "device_type": a few things require it, nothing else should use it. >>> >>> If that is the agreed approach in devicetree arena I am fine with it. I >>> have been unaware of this and just looked at the suggestion from Jonas >>> seeing a solution to the problem at hand. >> >> I don't think it has been discussed or decided before as the question >> has not come up, so for now this is my personal view. Maybe one of >> the devicetree maintainers can comment on this. > > Back from vacation and getting caught up. > > I agree with Arnd here. In my view model is the OEM branding on the > device, compatible is the h/w. If you have different firmware related > files, that goes beyond OEM branding. Thanks, Rob We are talking about hardware variants here. So using the model property is off the table. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On Thu, Jul 07, 2016 at 10:46:28AM +0200, Arnd Bergmann wrote: > On Wednesday, July 6, 2016 9:19:41 PM CEST Arend Van Spriel wrote: > > On 6-7-2016 15:42, Arnd Bergmann wrote: > > > On Wednesday, July 6, 2016 10:08:55 AM CEST Arend Van Spriel wrote: > > >> On Tue, Jul 5, 2016 at 3:43 PM, Arnd Bergmann wrote: > > > All existing uses of the model property in arch/arm/boot/dts and most of > > > the ones in arch/powerpc/boot/dts are against the intended usage in > > > one way or another, but adding different kind of incorrect usage won't > > > improve that. > > > > > > The only way I can see the model property being used correctly would > > > be to have it match the first entry in the compatible property, but > > > that is completely redundant, so we tend to omit it, except for the > > > root node in which it is required. For the root node however, the > > > historic practice that has crept in on ARM is to put something completely > > > different in there, which is a human-readable description of the > > > machine rather than something we can use as a unique indentifier. > > > > > > I'd just consider the "model" property burned, and not use it for anything > > > that doesn't already use it, just like we handle "device_type": a few > > > things require it, nothing else should use it. > > > > If that is the agreed approach in devicetree arena I am fine with it. I > > have been unaware of this and just looked at the suggestion from Jonas > > seeing a solution to the problem at hand. > > I don't think it has been discussed or decided before as the question > has not come up, so for now this is my personal view. Maybe one of > the devicetree maintainers can comment on this. Back from vacation and getting caught up. I agree with Arnd here. In my view model is the OEM branding on the device, compatible is the h/w. If you have different firmware related files, that goes beyond OEM branding. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm, nvram_file_name dt property
On Thursday, July 7, 2016 11:16:59 AM CEST Arend Van Spriel wrote: > On 7-7-2016 10:46, Arnd Bergmann wrote: > > On Wednesday, July 6, 2016 9:19:41 PM CEST Arend Van Spriel wrote: > >> On 6-7-2016 15:42, Arnd Bergmann wrote: > >>> On Wednesday, July 6, 2016 10:08:55 AM CEST Arend Van Spriel wrote: > On Tue, Jul 5, 2016 at 3:43 PM, Arnd Bergmann wrote: > > I'm a bit confused by the interdependencies now. You say that there are > > board ID/rev values stored in OTP. What exactly prevents us from just > > using those to generate a file name to look up the nvram file on disk > > for the remaining values? > > > > Do we require the firmware to be running before we can read the OTP, > > or are there cases where the board ID in OTP is wrong or missing? > > Well, both. Primarily we need firmware running to get the info. If board > ID is missing in OTP the value from nvram file is used. If board ID in > OTP is wrong we are screwed Ok. > > If we can't rely on OTP for one of those reasons and instead add two > > properties for boardtype/boardrev, I don't think that requires a > > change of the compatible string, we would just document those two > > as optional properties. > > Right. I suppose the bindings update and driver update should preferably > be in the same kernel release although bindings are supposedly OS agnostic. It's a one-way dependency, we shouldn't add the kernel code handling the property without having agreed on and updated the binding first. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On 7-7-2016 10:46, Arnd Bergmann wrote: > On Wednesday, July 6, 2016 9:19:41 PM CEST Arend Van Spriel wrote: >> On 6-7-2016 15:42, Arnd Bergmann wrote: >>> On Wednesday, July 6, 2016 10:08:55 AM CEST Arend Van Spriel wrote: On Tue, Jul 5, 2016 at 3:43 PM, Arnd Bergmann wrote: >>> All existing uses of the model property in arch/arm/boot/dts and most of >>> the ones in arch/powerpc/boot/dts are against the intended usage in >>> one way or another, but adding different kind of incorrect usage won't >>> improve that. >>> >>> The only way I can see the model property being used correctly would >>> be to have it match the first entry in the compatible property, but >>> that is completely redundant, so we tend to omit it, except for the >>> root node in which it is required. For the root node however, the >>> historic practice that has crept in on ARM is to put something completely >>> different in there, which is a human-readable description of the >>> machine rather than something we can use as a unique indentifier. >>> >>> I'd just consider the "model" property burned, and not use it for anything >>> that doesn't already use it, just like we handle "device_type": a few >>> things require it, nothing else should use it. >> >> If that is the agreed approach in devicetree arena I am fine with it. I >> have been unaware of this and just looked at the suggestion from Jonas >> seeing a solution to the problem at hand. > > I don't think it has been discussed or decided before as the question > has not come up, so for now this is my personal view. Maybe one of > the devicetree maintainers can comment on this. > >>> I agree with your point that the "compatible" property in case of brcmfmac >>> is really odd because it is not required to identify the hardware when >>> the SDIO device identification is sufficient, and we just need it to guard >>> the definition of the other properties. However it seems that now we >>> actually do need to identify the hardware because we cannot read the >>> board ID and revision that is supposed to come from nvram but also needed >>> to read the nvram contents from a file. >> >> The board ID and rev in the nvram may not be used if the device has >> these stored in OTP. However, the problem is that the device need >> firmware+nvram loaded into it before we can start the firmware on the >> device. Now if we were to add boardtype and boardrev properties in the >> binding to identify the hardware, I suppose a new compatible value would >> be required. > > I'm a bit confused by the interdependencies now. You say that there are > board ID/rev values stored in OTP. What exactly prevents us from just > using those to generate a file name to look up the nvram file on disk > for the remaining values? > > Do we require the firmware to be running before we can read the OTP, > or are there cases where the board ID in OTP is wrong or missing? Well, both. Primarily we need firmware running to get the info. If board ID is missing in OTP the value from nvram file is used. If board ID in OTP is wrong we are screwed :-p > If we can't rely on OTP for one of those reasons and instead add two > properties for boardtype/boardrev, I don't think that requires a > change of the compatible string, we would just document those two > as optional properties. Right. I suppose the bindings update and driver update should preferably be in the same kernel release although bindings are supposedly OS agnostic. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On Wednesday, July 6, 2016 9:19:41 PM CEST Arend Van Spriel wrote: > On 6-7-2016 15:42, Arnd Bergmann wrote: > > On Wednesday, July 6, 2016 10:08:55 AM CEST Arend Van Spriel wrote: > >> On Tue, Jul 5, 2016 at 3:43 PM, Arnd Bergmann wrote: > > All existing uses of the model property in arch/arm/boot/dts and most of > > the ones in arch/powerpc/boot/dts are against the intended usage in > > one way or another, but adding different kind of incorrect usage won't > > improve that. > > > > The only way I can see the model property being used correctly would > > be to have it match the first entry in the compatible property, but > > that is completely redundant, so we tend to omit it, except for the > > root node in which it is required. For the root node however, the > > historic practice that has crept in on ARM is to put something completely > > different in there, which is a human-readable description of the > > machine rather than something we can use as a unique indentifier. > > > > I'd just consider the "model" property burned, and not use it for anything > > that doesn't already use it, just like we handle "device_type": a few > > things require it, nothing else should use it. > > If that is the agreed approach in devicetree arena I am fine with it. I > have been unaware of this and just looked at the suggestion from Jonas > seeing a solution to the problem at hand. I don't think it has been discussed or decided before as the question has not come up, so for now this is my personal view. Maybe one of the devicetree maintainers can comment on this. > > I agree with your point that the "compatible" property in case of brcmfmac > > is really odd because it is not required to identify the hardware when > > the SDIO device identification is sufficient, and we just need it to guard > > the definition of the other properties. However it seems that now we > > actually do need to identify the hardware because we cannot read the > > board ID and revision that is supposed to come from nvram but also needed > > to read the nvram contents from a file. > > The board ID and rev in the nvram may not be used if the device has > these stored in OTP. However, the problem is that the device need > firmware+nvram loaded into it before we can start the firmware on the > device. Now if we were to add boardtype and boardrev properties in the > binding to identify the hardware, I suppose a new compatible value would > be required. I'm a bit confused by the interdependencies now. You say that there are board ID/rev values stored in OTP. What exactly prevents us from just using those to generate a file name to look up the nvram file on disk for the remaining values? Do we require the firmware to be running before we can read the OTP, or are there cases where the board ID in OTP is wrong or missing? If we can't rely on OTP for one of those reasons and instead add two properties for boardtype/boardrev, I don't think that requires a change of the compatible string, we would just document those two as optional properties. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On 6-7-2016 15:42, Arnd Bergmann wrote: > On Wednesday, July 6, 2016 10:08:55 AM CEST Arend Van Spriel wrote: >> On Tue, Jul 5, 2016 at 3:43 PM, Arnd Bergmann wrote: >>> On Monday, July 4, 2016 8:36:05 PM CEST Arend van Spriel wrote: On 04-07-16 16:54, Arnd Bergmann wrote: > On Monday, July 4, 2016 11:08:38 AM CEST Arend Van Spriel wrote: > > In drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c I already see > over a dozen different chips being supported, bcm4329 is only one of > them. In particular, there seem to be some that have various modules: > > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0x001F, 43241B0), > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0x0020, 43241B4), > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0xFFC0, 43241B5), > > So if you have a bcm43241, that compatible string probably should > include both brcm,bcm43241-b4-fmac and brcm,bcm43241-fmac, possibly also > brcm,bcm4329-fmac, to show that it is a superset of the programming > interface of that one. Hi Arnd, I have to disagree here. The compatible string "brcm,bcm4329-fmac" is chosen as the bcm4329 chip was the first supported and we never added others as there is no other programming required. For all supported devices the same device tree properties apply and are handled the same. As such there is no need to come up with a new compatible string. >>> >>> Generally speaking, the way that the compatible strings work is that you >>> add a new one whenever you get a new piece of hardware, and you can leave >>> the first one as a fallback so you don't have to change the driver. >>> >>> Adding the new string for the actual device is important though, >>> since you might only discover later that they are not 100% compatible >>> and that you in fact need to know the difference. >>> >>> For discoverable buses like sdio or usb, it may actually be better >>> to not identify the device through the compatible property at all, >>> and instead use a string that is generated from the actual identifier >>> as the primary key, as e.g. documented in >>> Documentation/devicetree/bindings/usb/usb-device.txt >> >> Well, that is my point. We do not need to identify the device here. We >> can obtain that info, ie. chip id and revision, from the device itself >> as our wifi chips have a discoverable AXI backplane. What is missing >> is that we have no way to tell what board/module this device is >> integrated on, which makes it impossible to select the correct nvram >> file. The model property can fill that gap just nicely. > > We have multiple inconsistencies here, and it's not this driver that is > particularly messed up, but I think using the model property here > would make it worse: > > All existing uses of the model property in arch/arm/boot/dts and most of > the ones in arch/powerpc/boot/dts are against the intended usage in > one way or another, but adding different kind of incorrect usage won't > improve that. > > The only way I can see the model property being used correctly would > be to have it match the first entry in the compatible property, but > that is completely redundant, so we tend to omit it, except for the > root node in which it is required. For the root node however, the > historic practice that has crept in on ARM is to put something completely > different in there, which is a human-readable description of the > machine rather than something we can use as a unique indentifier. > > I'd just consider the "model" property burned, and not use it for anything > that doesn't already use it, just like we handle "device_type": a few > things require it, nothing else should use it. If that is the agreed approach in devicetree arena I am fine with it. I have been unaware of this and just looked at the suggestion from Jonas seeing a solution to the problem at hand. > I agree with your point that the "compatible" property in case of brcmfmac > is really odd because it is not required to identify the hardware when > the SDIO device identification is sufficient, and we just need it to guard > the definition of the other properties. However it seems that now we > actually do need to identify the hardware because we cannot read the > board ID and revision that is supposed to come from nvram but also needed > to read the nvram contents from a file. The board ID and rev in the nvram may not be used if the device has these stored in OTP. However, the problem is that the device need firmware+nvram loaded into it before we can start the firmware on the device. Now if we were to add boardtype and boardrev properties in the binding to identify the hardware, I suppose a new compatible value would be required. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majo
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On Wednesday, July 6, 2016 10:08:55 AM CEST Arend Van Spriel wrote: > On Tue, Jul 5, 2016 at 3:43 PM, Arnd Bergmann wrote: > > On Monday, July 4, 2016 8:36:05 PM CEST Arend van Spriel wrote: > >> On 04-07-16 16:54, Arnd Bergmann wrote: > >> > On Monday, July 4, 2016 11:08:38 AM CEST Arend Van Spriel wrote: > >> > > >> > In drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c I already see > >> > over a dozen different chips being supported, bcm4329 is only one of > >> > them. In particular, there seem to be some that have various modules: > >> > > >> > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0x001F, 43241B0), > >> > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0x0020, 43241B4), > >> > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0xFFC0, 43241B5), > >> > > >> > So if you have a bcm43241, that compatible string probably should > >> > include both brcm,bcm43241-b4-fmac and brcm,bcm43241-fmac, possibly also > >> > brcm,bcm4329-fmac, to show that it is a superset of the programming > >> > interface of that one. > >> > >> Hi Arnd, > >> > >> I have to disagree here. The compatible string "brcm,bcm4329-fmac" is > >> chosen as the bcm4329 chip was the first supported and we never added > >> others as there is no other programming required. For all supported > >> devices the same device tree properties apply and are handled the same. > >> As such there is no need to come up with a new compatible string. > > > > Generally speaking, the way that the compatible strings work is that you > > add a new one whenever you get a new piece of hardware, and you can leave > > the first one as a fallback so you don't have to change the driver. > > > > Adding the new string for the actual device is important though, > > since you might only discover later that they are not 100% compatible > > and that you in fact need to know the difference. > > > > For discoverable buses like sdio or usb, it may actually be better > > to not identify the device through the compatible property at all, > > and instead use a string that is generated from the actual identifier > > as the primary key, as e.g. documented in > > Documentation/devicetree/bindings/usb/usb-device.txt > > Well, that is my point. We do not need to identify the device here. We > can obtain that info, ie. chip id and revision, from the device itself > as our wifi chips have a discoverable AXI backplane. What is missing > is that we have no way to tell what board/module this device is > integrated on, which makes it impossible to select the correct nvram > file. The model property can fill that gap just nicely. We have multiple inconsistencies here, and it's not this driver that is particularly messed up, but I think using the model property here would make it worse: All existing uses of the model property in arch/arm/boot/dts and most of the ones in arch/powerpc/boot/dts are against the intended usage in one way or another, but adding different kind of incorrect usage won't improve that. The only way I can see the model property being used correctly would be to have it match the first entry in the compatible property, but that is completely redundant, so we tend to omit it, except for the root node in which it is required. For the root node however, the historic practice that has crept in on ARM is to put something completely different in there, which is a human-readable description of the machine rather than something we can use as a unique indentifier. I'd just consider the "model" property burned, and not use it for anything that doesn't already use it, just like we handle "device_type": a few things require it, nothing else should use it. I agree with your point that the "compatible" property in case of brcmfmac is really odd because it is not required to identify the hardware when the SDIO device identification is sufficient, and we just need it to guard the definition of the other properties. However it seems that now we actually do need to identify the hardware because we cannot read the board ID and revision that is supposed to come from nvram but also needed to read the nvram contents from a file. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On Tue, Jul 5, 2016 at 3:43 PM, Arnd Bergmann wrote: > On Monday, July 4, 2016 8:36:05 PM CEST Arend van Spriel wrote: >> On 04-07-16 16:54, Arnd Bergmann wrote: >> > On Monday, July 4, 2016 11:08:38 AM CEST Arend Van Spriel wrote: >> > >> > In drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c I already see >> > over a dozen different chips being supported, bcm4329 is only one of >> > them. In particular, there seem to be some that have various modules: >> > >> > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0x001F, 43241B0), >> > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0x0020, 43241B4), >> > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0xFFC0, 43241B5), >> > >> > So if you have a bcm43241, that compatible string probably should >> > include both brcm,bcm43241-b4-fmac and brcm,bcm43241-fmac, possibly also >> > brcm,bcm4329-fmac, to show that it is a superset of the programming >> > interface of that one. >> >> Hi Arnd, >> >> I have to disagree here. The compatible string "brcm,bcm4329-fmac" is >> chosen as the bcm4329 chip was the first supported and we never added >> others as there is no other programming required. For all supported >> devices the same device tree properties apply and are handled the same. >> As such there is no need to come up with a new compatible string. > > Generally speaking, the way that the compatible strings work is that you > add a new one whenever you get a new piece of hardware, and you can leave > the first one as a fallback so you don't have to change the driver. > > Adding the new string for the actual device is important though, > since you might only discover later that they are not 100% compatible > and that you in fact need to know the difference. > > For discoverable buses like sdio or usb, it may actually be better > to not identify the device through the compatible property at all, > and instead use a string that is generated from the actual identifier > as the primary key, as e.g. documented in > Documentation/devicetree/bindings/usb/usb-device.txt Well, that is my point. We do not need to identify the device here. We can obtain that info, ie. chip id and revision, from the device itself as our wifi chips have a discoverable AXI backplane. What is missing is that we have no way to tell what board/module this device is integrated on, which makes it impossible to select the correct nvram file. The model property can fill that gap just nicely. Regards, Arend > The mmc binding is less clear about that, and we may want to correct > that. In fact, the example in > Documentation/devicetree/bindings/mmc/mmc.txt even lists an invalid > compatible string, so that is even worse. > > Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On Monday, July 4, 2016 8:36:05 PM CEST Arend van Spriel wrote: > On 04-07-16 16:54, Arnd Bergmann wrote: > > On Monday, July 4, 2016 11:08:38 AM CEST Arend Van Spriel wrote: > > > > In drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c I already see > > over a dozen different chips being supported, bcm4329 is only one of > > them. In particular, there seem to be some that have various modules: > > > > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0x001F, 43241B0), > > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0x0020, 43241B4), > > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0xFFC0, 43241B5), > > > > So if you have a bcm43241, that compatible string probably should > > include both brcm,bcm43241-b4-fmac and brcm,bcm43241-fmac, possibly also > > brcm,bcm4329-fmac, to show that it is a superset of the programming > > interface of that one. > > Hi Arnd, > > I have to disagree here. The compatible string "brcm,bcm4329-fmac" is > chosen as the bcm4329 chip was the first supported and we never added > others as there is no other programming required. For all supported > devices the same device tree properties apply and are handled the same. > As such there is no need to come up with a new compatible string. Generally speaking, the way that the compatible strings work is that you add a new one whenever you get a new piece of hardware, and you can leave the first one as a fallback so you don't have to change the driver. Adding the new string for the actual device is important though, since you might only discover later that they are not 100% compatible and that you in fact need to know the difference. For discoverable buses like sdio or usb, it may actually be better to not identify the device through the compatible property at all, and instead use a string that is generated from the actual identifier as the primary key, as e.g. documented in Documentation/devicetree/bindings/usb/usb-device.txt The mmc binding is less clear about that, and we may want to correct that. In fact, the example in Documentation/devicetree/bindings/mmc/mmc.txt even lists an invalid compatible string, so that is even worse. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On 04-07-16 16:54, Arnd Bergmann wrote: > On Monday, July 4, 2016 11:08:38 AM CEST Arend Van Spriel wrote: >> On 4-7-2016 10:55, Arnd Bergmann wrote: >>> On Monday, July 4, 2016 10:41:20 AM CEST Arend Van Spriel wrote: On 2-7-2016 23:30, Arnd Bergmann wrote: > On Saturday, July 2, 2016 8:20:35 PM CEST Arend Van Spriel wrote: >>> If you want a separate property, then I repeat my very first >>> suggestion, the well defined model property. >>> e.g. >>> >>> brcmf@0 { >>> model = "ampak,ap6210"; >>> compatible = "brcm,bcm4329-fmac"; >>> ... >>> }; >>> >>> All device nodes may have a model property, not just the top "machine" >>> one. >> >> I heard you the first time I just was not sure what the implications >> would be to use it. Hence I suggested a vendor specific property. >> However, looking up and reading the definition in ePAPRv1.1 I suppose it >> is fine to use the model property: >> >> Property: model >> Value type: >> Description: >> The model property value is a that specifies the manufacturer’s >> model number of the device. >> >> The recommended format is: “manufacturer,model”, where manufacturer is a >> string describing the name of the manufacturer (such as a stock ticker >> symbol), and model specifies the model number. > > The model property is very similar to compatible, except that there is > only one entry rather than a list of entries from most specific to > most generic. They seem very similar, but I think there is a conceptual difference. The compatible property is mainly used to select the appropriate driver and as such the property is typically ignored by device drivers. Probably there are exceptions to be found. > I think by writing the above example as > > compatible = "ampak,ap6210", "brcm,bcm4329-fmac"; > > we can provide the same functionality in a slightly simpler way, the > driver > then just goes on to look for the nvram file for each entry in sequence > until > it finds one. Not sure why this would be simpler. Why would traversing the compatible string be simpler than handling the model property if present and otherwise fallback to the default nvram naming. >>> >>> Because you have to walk the list anyway to find the other firmware files: >>> when you have a specialization of a device that requires listing both values >>> as compatible, the driver has no idea which of the entries to use, unless >>> you add a lookup table that adds more complexity. >> >> Currently, the brcmfmac bindings describe a single compatible string, >> ie. "brcm,bcm4329-fmac", which selects the driver/programming model. If >> that programming model supports "use model property if present, >> otherwise use default" there is nothing to traverse. The default way in >> the driver to determine firmware and nvram filename already has a lookup >> table which uses the chip id and chip revision as key, which are read >> from the device. > > In drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c I already see > over a dozen different chips being supported, bcm4329 is only one of > them. In particular, there seem to be some that have various modules: > > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0x001F, 43241B0), > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0x0020, 43241B4), > BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0xFFC0, 43241B5), > > So if you have a bcm43241, that compatible string probably should > include both brcm,bcm43241-b4-fmac and brcm,bcm43241-fmac, possibly also > brcm,bcm4329-fmac, to show that it is a superset of the programming > interface of that one. Hi Arnd, I have to disagree here. The compatible string "brcm,bcm4329-fmac" is chosen as the bcm4329 chip was the first supported and we never added others as there is no other programming required. For all supported devices the same device tree properties apply and are handled the same. As such there is no need to come up with a new compatible string. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On Monday, July 4, 2016 11:08:38 AM CEST Arend Van Spriel wrote: > On 4-7-2016 10:55, Arnd Bergmann wrote: > > On Monday, July 4, 2016 10:41:20 AM CEST Arend Van Spriel wrote: > >> On 2-7-2016 23:30, Arnd Bergmann wrote: > >>> On Saturday, July 2, 2016 8:20:35 PM CEST Arend Van Spriel wrote: > > If you want a separate property, then I repeat my very first > > suggestion, the well defined model property. > > e.g. > > > > brcmf@0 { > > model = "ampak,ap6210"; > > compatible = "brcm,bcm4329-fmac"; > > ... > > }; > > > > All device nodes may have a model property, not just the top "machine" > > one. > > I heard you the first time I just was not sure what the implications > would be to use it. Hence I suggested a vendor specific property. > However, looking up and reading the definition in ePAPRv1.1 I suppose it > is fine to use the model property: > > Property: model > Value type: > Description: > The model property value is a that specifies the manufacturer’s > model number of the device. > > The recommended format is: “manufacturer,model”, where manufacturer is a > string describing the name of the manufacturer (such as a stock ticker > symbol), and model specifies the model number. > >>> > >>> The model property is very similar to compatible, except that there is > >>> only one entry rather than a list of entries from most specific to > >>> most generic. > >> > >> They seem very similar, but I think there is a conceptual difference. > >> The compatible property is mainly used to select the appropriate driver > >> and as such the property is typically ignored by device drivers. > >> Probably there are exceptions to be found. > >> > >>> I think by writing the above example as > >>> > >>> compatible = "ampak,ap6210", "brcm,bcm4329-fmac"; > >>> > >>> we can provide the same functionality in a slightly simpler way, the > >>> driver > >>> then just goes on to look for the nvram file for each entry in sequence > >>> until > >>> it finds one. > >> > >> Not sure why this would be simpler. Why would traversing the compatible > >> string be simpler than handling the model property if present and > >> otherwise fallback to the default nvram naming. > > > > Because you have to walk the list anyway to find the other firmware files: > > when you have a specialization of a device that requires listing both values > > as compatible, the driver has no idea which of the entries to use, unless > > you add a lookup table that adds more complexity. > > Currently, the brcmfmac bindings describe a single compatible string, > ie. "brcm,bcm4329-fmac", which selects the driver/programming model. If > that programming model supports "use model property if present, > otherwise use default" there is nothing to traverse. The default way in > the driver to determine firmware and nvram filename already has a lookup > table which uses the chip id and chip revision as key, which are read > from the device. In drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c I already see over a dozen different chips being supported, bcm4329 is only one of them. In particular, there seem to be some that have various modules: BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0x001F, 43241B0), BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0x0020, 43241B4), BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43241_CHIP_ID, 0xFFC0, 43241B5), So if you have a bcm43241, that compatible string probably should include both brcm,bcm43241-b4-fmac and brcm,bcm43241-fmac, possibly also brcm,bcm4329-fmac, to show that it is a superset of the programming interface of that one. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On 4-7-2016 10:55, Arnd Bergmann wrote: > On Monday, July 4, 2016 10:41:20 AM CEST Arend Van Spriel wrote: >> On 2-7-2016 23:30, Arnd Bergmann wrote: >>> On Saturday, July 2, 2016 8:20:35 PM CEST Arend Van Spriel wrote: > If you want a separate property, then I repeat my very first > suggestion, the well defined model property. > e.g. > > brcmf@0 { > model = "ampak,ap6210"; > compatible = "brcm,bcm4329-fmac"; > ... > }; > > All device nodes may have a model property, not just the top "machine" > one. I heard you the first time I just was not sure what the implications would be to use it. Hence I suggested a vendor specific property. However, looking up and reading the definition in ePAPRv1.1 I suppose it is fine to use the model property: Property: model Value type: Description: The model property value is a that specifies the manufacturer’s model number of the device. The recommended format is: “manufacturer,model”, where manufacturer is a string describing the name of the manufacturer (such as a stock ticker symbol), and model specifies the model number. >>> >>> The model property is very similar to compatible, except that there is >>> only one entry rather than a list of entries from most specific to >>> most generic. >> >> They seem very similar, but I think there is a conceptual difference. >> The compatible property is mainly used to select the appropriate driver >> and as such the property is typically ignored by device drivers. >> Probably there are exceptions to be found. >> >>> I think by writing the above example as >>> >>> compatible = "ampak,ap6210", "brcm,bcm4329-fmac"; >>> >>> we can provide the same functionality in a slightly simpler way, the driver >>> then just goes on to look for the nvram file for each entry in sequence >>> until >>> it finds one. >> >> Not sure why this would be simpler. Why would traversing the compatible >> string be simpler than handling the model property if present and >> otherwise fallback to the default nvram naming. > > Because you have to walk the list anyway to find the other firmware files: > when you have a specialization of a device that requires listing both values > as compatible, the driver has no idea which of the entries to use, unless > you add a lookup table that adds more complexity. Currently, the brcmfmac bindings describe a single compatible string, ie. "brcm,bcm4329-fmac", which selects the driver/programming model. If that programming model supports "use model property if present, otherwise use default" there is nothing to traverse. The default way in the driver to determine firmware and nvram filename already has a lookup table which uses the chip id and chip revision as key, which are read from the device. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On Monday, July 4, 2016 10:41:20 AM CEST Arend Van Spriel wrote: > On 2-7-2016 23:30, Arnd Bergmann wrote: > > On Saturday, July 2, 2016 8:20:35 PM CEST Arend Van Spriel wrote: > >>> If you want a separate property, then I repeat my very first > >>> suggestion, the well defined model property. > >>> e.g. > >>> > >>> brcmf@0 { > >>> model = "ampak,ap6210"; > >>> compatible = "brcm,bcm4329-fmac"; > >>> ... > >>> }; > >>> > >>> All device nodes may have a model property, not just the top "machine" > >>> one. > >> > >> I heard you the first time I just was not sure what the implications > >> would be to use it. Hence I suggested a vendor specific property. > >> However, looking up and reading the definition in ePAPRv1.1 I suppose it > >> is fine to use the model property: > >> > >> Property: model > >> Value type: > >> Description: > >> The model property value is a that specifies the manufacturer’s > >> model number of the device. > >> > >> The recommended format is: “manufacturer,model”, where manufacturer is a > >> string describing the name of the manufacturer (such as a stock ticker > >> symbol), and model specifies the model number. > > > > The model property is very similar to compatible, except that there is > > only one entry rather than a list of entries from most specific to > > most generic. > > They seem very similar, but I think there is a conceptual difference. > The compatible property is mainly used to select the appropriate driver > and as such the property is typically ignored by device drivers. > Probably there are exceptions to be found. > > > I think by writing the above example as > > > > compatible = "ampak,ap6210", "brcm,bcm4329-fmac"; > > > > we can provide the same functionality in a slightly simpler way, the driver > > then just goes on to look for the nvram file for each entry in sequence > > until > > it finds one. > > Not sure why this would be simpler. Why would traversing the compatible > string be simpler than handling the model property if present and > otherwise fallback to the default nvram naming. Because you have to walk the list anyway to find the other firmware files: when you have a specialization of a device that requires listing both values as compatible, the driver has no idea which of the entries to use, unless you add a lookup table that adds more complexity. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On 2-7-2016 23:30, Arnd Bergmann wrote: > On Saturday, July 2, 2016 8:20:35 PM CEST Arend Van Spriel wrote: >>> If you want a separate property, then I repeat my very first >>> suggestion, the well defined model property. >>> e.g. >>> >>> brcmf@0 { >>> model = "ampak,ap6210"; >>> compatible = "brcm,bcm4329-fmac"; >>> ... >>> }; >>> >>> All device nodes may have a model property, not just the top "machine" one. >> >> I heard you the first time I just was not sure what the implications >> would be to use it. Hence I suggested a vendor specific property. >> However, looking up and reading the definition in ePAPRv1.1 I suppose it >> is fine to use the model property: >> >> Property: model >> Value type: >> Description: >> The model property value is a that specifies the manufacturer’s >> model number of the device. >> >> The recommended format is: “manufacturer,model”, where manufacturer is a >> string describing the name of the manufacturer (such as a stock ticker >> symbol), and model specifies the model number. > > The model property is very similar to compatible, except that there is > only one entry rather than a list of entries from most specific to > most generic. They seem very similar, but I think there is a conceptual difference. The compatible property is mainly used to select the appropriate driver and as such the property is typically ignored by device drivers. Probably there are exceptions to be found. > I think by writing the above example as > > compatible = "ampak,ap6210", "brcm,bcm4329-fmac"; > > we can provide the same functionality in a slightly simpler way, the driver > then just goes on to look for the nvram file for each entry in sequence until > it finds one. Not sure why this would be simpler. Why would traversing the compatible string be simpler than handling the model property if present and otherwise fallback to the default nvram naming. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On Saturday, July 2, 2016 8:20:35 PM CEST Arend Van Spriel wrote: > > If you want a separate property, then I repeat my very first > > suggestion, the well defined model property. > > e.g. > > > > brcmf@0 { > > model = "ampak,ap6210"; > > compatible = "brcm,bcm4329-fmac"; > > ... > > }; > > > > All device nodes may have a model property, not just the top "machine" one. > > I heard you the first time I just was not sure what the implications > would be to use it. Hence I suggested a vendor specific property. > However, looking up and reading the definition in ePAPRv1.1 I suppose it > is fine to use the model property: > > Property: model > Value type: > Description: > The model property value is a that specifies the manufacturer’s > model number of the device. > > The recommended format is: “manufacturer,model”, where manufacturer is a > string describing the name of the manufacturer (such as a stock ticker > symbol), and model specifies the model number. The model property is very similar to compatible, except that there is only one entry rather than a list of entries from most specific to most generic. I think by writing the above example as compatible = "ampak,ap6210", "brcm,bcm4329-fmac"; we can provide the same functionality in a slightly simpler way, the driver then just goes on to look for the nvram file for each entry in sequence until it finds one. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On 1-7-2016 10:58, Jonas Gorski wrote: > Hi, > > On 30 June 2016 at 21:23, Arend Van Spriel > wrote: >> >> >> On 30-6-2016 13:31, Arnd Bergmann wrote: >>> On Thursday, June 30, 2016 12:25:15 PM CEST Hans de Goede wrote: > So then how about making use of a more specific compatible string? > > e.g. > > brcmf { > compatible = "foo,ap6210", "brcm,bcm4329-fmac"; > ... > }; > > and if the compatible has more than one element you request > FW_NAME_.txt as the nvram file. Or try each comptible (and > lastly no suffix) until you get a match. (AFAICT, this is what the > "model" property was originally intended for anyway, but almost nobody > did it right, and everyone put a user readable string into "model" for > boards instead of the ePAPR defined compatible string). Hmm, interesting idea. Not sure how easy / hard it will be to implement this, but from a dt binding point of view it seems elegant. Kalle, Arend, what do you think of this ? >> >> At first glance I like the suggestion, but this would mean updating the >> bindings document for each new wifi module that we want to add. Not a >> big problem, but it makes that I have a slight preference to using a >> property for it, eg. brcm,module = "ap6210"; > > If you want a separate property, then I repeat my very first > suggestion, the well defined model property. > e.g. > > brcmf@0 { > model = "ampak,ap6210"; > compatible = "brcm,bcm4329-fmac"; > ... > }; > > All device nodes may have a model property, not just the top "machine" one. I heard you the first time :-p I just was not sure what the implications would be to use it. Hence I suggested a vendor specific property. However, looking up and reading the definition in ePAPRv1.1 I suppose it is fine to use the model property: Property: model Value type: Description: The model property value is a that specifies the manufacturer’s model number of the device. The recommended format is: “manufacturer,model”, where manufacturer is a string describing the name of the manufacturer (such as a stock ticker symbol), and model specifies the model number. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Jonas Gorski writes: > Hi, > > On 30 June 2016 at 21:23, Arend Van Spriel > wrote: >> >> >> On 30-6-2016 13:31, Arnd Bergmann wrote: >>> On Thursday, June 30, 2016 12:25:15 PM CEST Hans de Goede wrote: > So then how about making use of a more specific compatible string? > > e.g. > > brcmf { > compatible = "foo,ap6210", "brcm,bcm4329-fmac"; > ... > }; > > and if the compatible has more than one element you request > FW_NAME_.txt as the nvram file. Or try each comptible (and > lastly no suffix) until you get a match. (AFAICT, this is what the > "model" property was originally intended for anyway, but almost nobody > did it right, and everyone put a user readable string into "model" for > boards instead of the ePAPR defined compatible string). Hmm, interesting idea. Not sure how easy / hard it will be to implement this, but from a dt binding point of view it seems elegant. Kalle, Arend, what do you think of this ? >> >> At first glance I like the suggestion, but this would mean updating the >> bindings document for each new wifi module that we want to add. Not a >> big problem, but it makes that I have a slight preference to using a >> property for it, eg. brcm,module = "ap6210"; > > If you want a separate property, then I repeat my very first > suggestion, the well defined model property. > e.g. > > brcmf@0 { > model = "ampak,ap6210"; > compatible = "brcm,bcm4329-fmac"; > ... > }; > > All device nodes may have a model property, not just the top "machine" one. I like this model property but I'm no DT expert. What others think about it, would it work? -- Kalle Valo -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Hi, On 30 June 2016 at 21:23, Arend Van Spriel wrote: > > > On 30-6-2016 13:31, Arnd Bergmann wrote: >> On Thursday, June 30, 2016 12:25:15 PM CEST Hans de Goede wrote: So then how about making use of a more specific compatible string? e.g. brcmf { compatible = "foo,ap6210", "brcm,bcm4329-fmac"; ... }; and if the compatible has more than one element you request FW_NAME_.txt as the nvram file. Or try each comptible (and lastly no suffix) until you get a match. (AFAICT, this is what the "model" property was originally intended for anyway, but almost nobody did it right, and everyone put a user readable string into "model" for boards instead of the ePAPR defined compatible string). >>> >>> Hmm, interesting idea. Not sure how easy / hard it will be to implement >>> this, but from a dt binding point of view it seems elegant. >>> >>> Kalle, Arend, what do you think of this ? > > At first glance I like the suggestion, but this would mean updating the > bindings document for each new wifi module that we want to add. Not a > big problem, but it makes that I have a slight preference to using a > property for it, eg. brcm,module = "ap6210"; If you want a separate property, then I repeat my very first suggestion, the well defined model property. e.g. brcmf@0 { model = "ampak,ap6210"; compatible = "brcm,bcm4329-fmac"; ... }; All device nodes may have a model property, not just the top "machine" one. Regards Jonas -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm, nvram_file_name dt property
On Thursday, June 30, 2016 9:23:56 PM CEST Arend Van Spriel wrote: > > On 30-6-2016 13:31, Arnd Bergmann wrote: > > On Thursday, June 30, 2016 12:25:15 PM CEST Hans de Goede wrote: > >>> So then how about making use of a more specific compatible string? > >>> > >>> e.g. > >>> > >>> brcmf { > >>> compatible = "foo,ap6210", "brcm,bcm4329-fmac"; > >>> ... > >>> }; > >>> > >>> and if the compatible has more than one element you request > >>> FW_NAME_.txt as the nvram file. Or try each comptible (and > >>> lastly no suffix) until you get a match. (AFAICT, this is what the > >>> "model" property was originally intended for anyway, but almost nobody > >>> did it right, and everyone put a user readable string into "model" for > >>> boards instead of the ePAPR defined compatible string). > >> > >> Hmm, interesting idea. Not sure how easy / hard it will be to implement > >> this, but from a dt binding point of view it seems elegant. > >> > >> Kalle, Arend, what do you think of this ? > > At first glance I like the suggestion, but this would mean updating the > bindings document for each new wifi module that we want to add. Not a > big problem, but it makes that I have a slight preference to using a > property for it, eg. brcm,module = "ap6210"; I think we can be a little relaxed with the requirement for updating the binding document here, as long as the binding lists all the strings that are understood by the driver itself and documents that there can be additional strings in it. In particular, documenting how a compatible string that is made up from the board id and revision should be unproblematic. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On 30-6-2016 13:31, Arnd Bergmann wrote: > On Thursday, June 30, 2016 12:25:15 PM CEST Hans de Goede wrote: >>> So then how about making use of a more specific compatible string? >>> >>> e.g. >>> >>> brcmf { >>> compatible = "foo,ap6210", "brcm,bcm4329-fmac"; >>> ... >>> }; >>> >>> and if the compatible has more than one element you request >>> FW_NAME_.txt as the nvram file. Or try each comptible (and >>> lastly no suffix) until you get a match. (AFAICT, this is what the >>> "model" property was originally intended for anyway, but almost nobody >>> did it right, and everyone put a user readable string into "model" for >>> boards instead of the ePAPR defined compatible string). >> >> Hmm, interesting idea. Not sure how easy / hard it will be to implement >> this, but from a dt binding point of view it seems elegant. >> >> Kalle, Arend, what do you think of this ? At first glance I like the suggestion, but this would mean updating the bindings document for each new wifi module that we want to add. Not a big problem, but it makes that I have a slight preference to using a property for it, eg. brcm,module = "ap6210"; > I think that's reasonable. Also, we have precedent for using things like > the boardid as part of the compatible string, which would help do what > Kalle suggested earlier with "nvram--.txt", > as we get that for free by trying out all the compatible strings. The suggestion from Kalle was to use the field in the nvram file, but things are a bit more complicated. The fields are only used if it is not stored on the device in OTP or SROM. The 43362 does not have a SROM, but it still has a small OTP storage if I am not mistaken. The brcmfmac exposes a revinfo file in debugfs containing boardtype and boardrev, but that is after starting the firmware. Still worth to check if those values match the values in the ap6210 nvram file. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On Thursday, June 30, 2016 12:25:15 PM CEST Hans de Goede wrote: > > So then how about making use of a more specific compatible string? > > > > e.g. > > > > brcmf { > > compatible = "foo,ap6210", "brcm,bcm4329-fmac"; > > ... > > }; > > > > and if the compatible has more than one element you request > > FW_NAME_.txt as the nvram file. Or try each comptible (and > > lastly no suffix) until you get a match. (AFAICT, this is what the > > "model" property was originally intended for anyway, but almost nobody > > did it right, and everyone put a user readable string into "model" for > > boards instead of the ePAPR defined compatible string). > > Hmm, interesting idea. Not sure how easy / hard it will be to implement > this, but from a dt binding point of view it seems elegant. > > Kalle, Arend, what do you think of this ? I think that's reasonable. Also, we have precedent for using things like the boardid as part of the compatible string, which would help do what Kalle suggested earlier with "nvram--.txt", as we get that for free by trying out all the compatible strings. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Hi, On 30 June 2016 at 12:04, Hans de Goede wrote: > Hi, > > > On 30-06-16 11:58, Kalle Valo wrote: >> >> Hans de Goede writes: >> >>> Hi, >>> >>> On 30-06-16 11:02, Kalle Valo wrote: Priit Laes writes: >> What is the size of this nvram file? As it's board specific, I wonder >> if we can simply include it inside of the DT verbatim. I remember >> doing that (in the pre-dtb days, on real open firmware) for the >> "spidernet" >> ethernet driver. > > > It contains a bit too much info: > > This is what CubieTruck requires: > > > http://dl.cubieboard.org/public/Cubieboard/benn/firmware/ap6210/nvram_ap6210.txt In the nvram file I see lots of identifiers: manfid=0x2d0 prodid=0x492 vendid=0x14e4 devid=0x4343 boardtype=0x0598 boardrev=0x1307 boardnum=777 Are any of these (or a combination of them) unique for each board type? What if one of these is provided through DT and then the driver could choose the nvram file based on the board id? Just another idea... >>> >>> >>> That would require updating a table in the driver every time a new >>> board comes out, I do not believe that that is a good solution. >> >> >> You don't necessarily need to have a table in the driver if you embed >> the id to the filename, for example something like this: >> >> nvram-0598-1307.txt >> nvram--.txt > > > Downside of this is, that as Arend already said, the nvram files are not > readily available. > > Typically the boards we are talking about are shipped with android, > and the mainline kernel bringup is done by a user, not the manufacturer. > > So the nvram file needs to be extracted from e.g. an android image, > having ap6210 in the filename allows the user to see that his module > (which is clearly labelled ap6210 is already supported, where as the > boardtype/boardrev magic numbers are a big unknown. > > So I believe that using a human readable string is better here. So then how about making use of a more specific compatible string? e.g. brcmf { compatible = "foo,ap6210", "brcm,bcm4329-fmac"; ... }; and if the compatible has more than one element you request FW_NAME_.txt as the nvram file. Or try each comptible (and lastly no suffix) until you get a match. (AFAICT, this is what the "model" property was originally intended for anyway, but almost nobody did it right, and everyone put a user readable string into "model" for boards instead of the ePAPR defined compatible string). Regards Jonas -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Hi, On 30-06-16 12:18, Jonas Gorski wrote: Hi, On 30 June 2016 at 12:04, Hans de Goede wrote: Hi, On 30-06-16 11:58, Kalle Valo wrote: Hans de Goede writes: Hi, On 30-06-16 11:02, Kalle Valo wrote: Priit Laes writes: What is the size of this nvram file? As it's board specific, I wonder if we can simply include it inside of the DT verbatim. I remember doing that (in the pre-dtb days, on real open firmware) for the "spidernet" ethernet driver. It contains a bit too much info: This is what CubieTruck requires: http://dl.cubieboard.org/public/Cubieboard/benn/firmware/ap6210/nvram_ap6210.txt In the nvram file I see lots of identifiers: manfid=0x2d0 prodid=0x492 vendid=0x14e4 devid=0x4343 boardtype=0x0598 boardrev=0x1307 boardnum=777 Are any of these (or a combination of them) unique for each board type? What if one of these is provided through DT and then the driver could choose the nvram file based on the board id? Just another idea... That would require updating a table in the driver every time a new board comes out, I do not believe that that is a good solution. You don't necessarily need to have a table in the driver if you embed the id to the filename, for example something like this: nvram-0598-1307.txt nvram--.txt Downside of this is, that as Arend already said, the nvram files are not readily available. Typically the boards we are talking about are shipped with android, and the mainline kernel bringup is done by a user, not the manufacturer. So the nvram file needs to be extracted from e.g. an android image, having ap6210 in the filename allows the user to see that his module (which is clearly labelled ap6210 is already supported, where as the boardtype/boardrev magic numbers are a big unknown. So I believe that using a human readable string is better here. So then how about making use of a more specific compatible string? e.g. brcmf { compatible = "foo,ap6210", "brcm,bcm4329-fmac"; ... }; and if the compatible has more than one element you request FW_NAME_.txt as the nvram file. Or try each comptible (and lastly no suffix) until you get a match. (AFAICT, this is what the "model" property was originally intended for anyway, but almost nobody did it right, and everyone put a user readable string into "model" for boards instead of the ePAPR defined compatible string). Hmm, interesting idea. Not sure how easy / hard it will be to implement this, but from a dt binding point of view it seems elegant. Kalle, Arend, what do you think of this ? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Hans de Goede writes: > Hi, > > On 30-06-16 11:02, Kalle Valo wrote: >> Priit Laes writes: >> What is the size of this nvram file? As it's board specific, I wonder if we can simply include it inside of the DT verbatim. I remember doing that (in the pre-dtb days, on real open firmware) for the "spidernet" ethernet driver. >>> >>> It contains a bit too much info: >>> >>> This is what CubieTruck requires: >>> >>> http://dl.cubieboard.org/public/Cubieboard/benn/firmware/ap6210/nvram_ap6210.txt >> >> In the nvram file I see lots of identifiers: >> >> manfid=0x2d0 >> prodid=0x492 >> vendid=0x14e4 >> devid=0x4343 >> boardtype=0x0598 >> boardrev=0x1307 >> boardnum=777 >> >> Are any of these (or a combination of them) unique for each board type? >> What if one of these is provided through DT and then the driver could >> choose the nvram file based on the board id? Just another idea... > > That would require updating a table in the driver every time a new > board comes out, I do not believe that that is a good solution. You don't necessarily need to have a table in the driver if you embed the id to the filename, for example something like this: nvram-0598-1307.txt nvram--.txt -- Kalle Valo -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Hi, On 30-06-16 11:58, Kalle Valo wrote: Hans de Goede writes: Hi, On 30-06-16 11:02, Kalle Valo wrote: Priit Laes writes: What is the size of this nvram file? As it's board specific, I wonder if we can simply include it inside of the DT verbatim. I remember doing that (in the pre-dtb days, on real open firmware) for the "spidernet" ethernet driver. It contains a bit too much info: This is what CubieTruck requires: http://dl.cubieboard.org/public/Cubieboard/benn/firmware/ap6210/nvram_ap6210.txt In the nvram file I see lots of identifiers: manfid=0x2d0 prodid=0x492 vendid=0x14e4 devid=0x4343 boardtype=0x0598 boardrev=0x1307 boardnum=777 Are any of these (or a combination of them) unique for each board type? What if one of these is provided through DT and then the driver could choose the nvram file based on the board id? Just another idea... That would require updating a table in the driver every time a new board comes out, I do not believe that that is a good solution. You don't necessarily need to have a table in the driver if you embed the id to the filename, for example something like this: nvram-0598-1307.txt nvram--.txt Downside of this is, that as Arend already said, the nvram files are not readily available. Typically the boards we are talking about are shipped with android, and the mainline kernel bringup is done by a user, not the manufacturer. So the nvram file needs to be extracted from e.g. an android image, having ap6210 in the filename allows the user to see that his module (which is clearly labelled ap6210 is already supported, where as the boardtype/boardrev magic numbers are a big unknown. So I believe that using a human readable string is better here. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Hi, On 29-06-16 20:51, 'Arend Van Spriel' via linux-sunxi wrote: On 29-6-2016 20:01, Hans de Goede wrote: Hi, On 29-06-16 19:00, Kalle Valo wrote: Hans de Goede writes: Hi, On 29-06-16 16:42, Jonas Gorski wrote: Hi, On 29 June 2016 at 16:04, 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. Directly defining a filename doesn't seem like a good OS-agnostic approach. Maybe an alternative would be to add a model-property to the nodes (this is allowed) and make brcmfmac to request "FWFILENAME-" as firmware if set? That would leave it to the OS on how the filename is set. It only defines the base-filename, not the entire path, how / where this file is searched for / loaded-from is then left up to the os It's still a bad idea. The filename, including the path, should be created in the driver. Can't you provide chipname (or similar) via device tree and then the driver can choose what image to use? No, the driver already does that, but this is not ... Can you tell more about the naming the firmware image, how does it work exactly? About firmware, this is about the nvram file which is board specific, rather then chip specific. The nvram filename is determined pretty much the same as the firmware filename, but indeed the nvram data is board specific. This is main reason why we do not submit nvram files to linux-firmware, because we simply do not have those files. Typical wifi devices will have some sort of non volatile storage on board to not only store the ethernet(mac) address, but also to contain e.g. info about the antenna gain so that the firmware and/or the driver can take the antenna gain into account and ensure that they never exceed the maximum allowed broadcast strength. However on some embedded devices there is no non-volatile storage for the wifi (for cost reasons) and instead this configuration info (which is board / pcb specific) is loaded in the form of a file which contains the contents which would normally be in the non-volatile storage. Since we are dealing with a per-board config-file here, which is loaded from the os filesystem we really need to specify a basename here as the list of possible boards is endless, so we cannot have a lookup table in the driver. As Jonas mentioned the general principle of device tree is to be agnostic with regards to OS and/or driver as you undoubtedly know. His proposal seems like a usable solution for your problem while complying to the device tree principle. So instead of overriding the default brcmfmac should modify it when dt specifies the "module" property, ie: no "module" in DT:nvram filename = brcm/brcmfmac43362-sdio.txt "module=ap6210" in DT:nvram filename = brcm/brcmfmac43362-ap6210.txt Yes that seems like a good solution, I will send a v2 implementing this. By the way, the example in the bindings file does not seem to specify a basename, but path+basename+fileext. fileext is always part of a file basename, but you're right that it does include a relative path because of the way the existing firmware files are organized under /lib/firmware under Linux and yes I'll admit that that is a bit os specific :) Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Hi, On 30-06-16 11:02, Kalle Valo wrote: Priit Laes writes: What is the size of this nvram file? As it's board specific, I wonder if we can simply include it inside of the DT verbatim. I remember doing that (in the pre-dtb days, on real open firmware) for the "spidernet" ethernet driver. It contains a bit too much info: This is what CubieTruck requires: http://dl.cubieboard.org/public/Cubieboard/benn/firmware/ap6210/nvram_ap6210.txt In the nvram file I see lots of identifiers: manfid=0x2d0 prodid=0x492 vendid=0x14e4 devid=0x4343 boardtype=0x0598 boardrev=0x1307 boardnum=777 Are any of these (or a combination of them) unique for each board type? What if one of these is provided through DT and then the driver could choose the nvram file based on the board id? Just another idea... That would require updating a table in the driver every time a new board comes out, I do not believe that that is a good solution. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Hi, On 30-06-16 10:46, Kalle Valo wrote: Arend Van Spriel writes: Since we are dealing with a per-board config-file here, which is loaded from the os filesystem we really need to specify a basename here as the list of possible boards is endless, so we cannot have a lookup table in the driver. As Jonas mentioned the general principle of device tree is to be agnostic with regards to OS and/or driver as you undoubtedly know. His proposal seems like a usable solution for your problem while complying to the device tree principle. So instead of overriding the default brcmfmac should modify it when dt specifies the "module" property, ie: no "module" in DT:nvram filename = brcm/brcmfmac43362-sdio.txt "module=ap6210" in DT:nvram filename = brcm/brcmfmac43362-ap6210.txt Just out of curiosity, what does "ap6210" exactly mean? I get that 43362 is the chip id, but not ap6210. Is it just an arbitrary name? It is more or less an arbitrary name, typically these sdio wifi chips are used as a pre-assembled module (a tiny pcb) with some things like the necessary crystal / resonator and various capacitors and resistors on there. The brcm based sdio wifi modules typically come with a metal shield cap which has a module type stamped on it, e.g. ap6210, ap6476, toc9002. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Priit Laes writes: >> What is the size of this nvram file? As it's board specific, I wonder >> if we can simply include it inside of the DT verbatim. I remember >> doing that (in the pre-dtb days, on real open firmware) for the >> "spidernet" >> ethernet driver. > > It contains a bit too much info: > > This is what CubieTruck requires: > > http://dl.cubieboard.org/public/Cubieboard/benn/firmware/ap6210/nvram_ap6210.txt In the nvram file I see lots of identifiers: manfid=0x2d0 prodid=0x492 vendid=0x14e4 devid=0x4343 boardtype=0x0598 boardrev=0x1307 boardnum=777 Are any of these (or a combination of them) unique for each board type? What if one of these is provided through DT and then the driver could choose the nvram file based on the board id? Just another idea... -- Kalle Valo -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Arend Van Spriel writes: >>> Typical wifi devices will have some sort of non volatile storage >>> on board to not only store the ethernet(mac) address, but also >>> to contain e.g. info about the antenna gain so that the firmware >>> and/or the driver can take the antenna gain into account and ensure >>> that they never exceed the maximum allowed broadcast strength. >>> >>> However on some embedded devices there is no non-volatile storage >>> for the wifi (for cost reasons) and instead this configuration info >>> (which is board / pcb specific) is loaded in the form of a >>> file which contains the contents which would normally be in the >>> non-volatile storage. >>> >>> Since we are dealing with a per-board config-file here, which is >>> loaded from the os filesystem we really need to specify a basename >>> here as the list of possible boards is endless, so we cannot >>> have a lookup table in the driver. > > As a note: For BT Marcel was playing with the idea of having a lookup > table on the file system which would be loaded by the driver. In ath10k we have a similar problem but in our case we can use the subsystem id to detect what board file to use, so it's not exactly same as yours. In our board-2.bin we have identifiers like this from which ath10k selects the correct board file: bus=pci,vendor=168c,device=003e,subsystem-vendor=168c,subsystem-device=334e bus=pci,vendor=168c,device=003e,subsystem-vendor=168c,subsystem-device=3360 bus=pci,vendor=168c,device=003e,subsystem-vendor=168c,subsystem-device=3361 -- Kalle Valo -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Arend Van Spriel writes: >> Since we are dealing with a per-board config-file here, which is >> loaded from the os filesystem we really need to specify a basename >> here as the list of possible boards is endless, so we cannot >> have a lookup table in the driver. > > As Jonas mentioned the general principle of device tree is to be > agnostic with regards to OS and/or driver as you undoubtedly know. His > proposal seems like a usable solution for your problem while complying > to the device tree principle. So instead of overriding the default > brcmfmac should modify it when dt specifies the "module" property, ie: > > no "module" in DT:nvram filename = brcm/brcmfmac43362-sdio.txt > "module=ap6210" in DT:nvram filename = brcm/brcmfmac43362-ap6210.txt Just out of curiosity, what does "ap6210" exactly mean? I get that 43362 is the chip id, but not ap6210. Is it just an arbitrary name? -- Kalle Valo -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On Wednesday, June 29, 2016 10:54:38 PM CEST Priit Laes wrote: > On Wed, 2016-06-29 at 21:33 +0200, Arnd Bergmann wrote: > > What is the size of this nvram file? As it's board specific, I wonder > > if we can simply include it inside of the DT verbatim. I remember > > doing that (in the pre-dtb days, on real open firmware) for the > > "spidernet" > > ethernet driver. > > It contains a bit too much info: > > This is what CubieTruck requires: > > http://dl.cubieboard.org/public/Cubieboard/benn/firmware/ap6210/nvram_a > p6210.txt Ah, I had not realized that this is a text based interface rather than a small binary blob with fixed offsets. On the other hand, each line in there could be translated easily into a separate DT property, and some of them (manfid/prodid, macaddr, ...) look like they directly correspond to properties we already have. As Arend said there is also the option of having a chip specific nvram file (brcmfmac43362-sdio.txt) as a fallback when there is no more specific module. How many of the lines in your example would actually differ between the two? Does this affect all of them, or just a subset that could be turned into a smaller set of DT properties? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On Wed, 2016-06-29 at 21:33 +0200, Arnd Bergmann wrote: > On Wednesday, June 29, 2016 8:51:44 PM CEST Arend Van Spriel wrote: > > > Typical wifi devices will have some sort of non volatile storage > > > on board to not only store the ethernet(mac) address, but also > > > to contain e.g. info about the antenna gain so that the firmware > > > and/or the driver can take the antenna gain into account and > > > ensure > > > that they never exceed the maximum allowed broadcast strength. > > > > > > However on some embedded devices there is no non-volatile storage > > > for the wifi (for cost reasons) and instead this configuration > > > info > > > (which is board / pcb specific) is loaded in the form of a > > > file which contains the contents which would normally be in the > > > non-volatile storage. > > > > > > Since we are dealing with a per-board config-file here, which is > > > loaded from the os filesystem we really need to specify a > > > basename > > > here as the list of possible boards is endless, so we cannot > > > have a lookup table in the driver. > > > > As Jonas mentioned the general principle of device tree is to be > > agnostic with regards to OS and/or driver as you undoubtedly know. > > His > > proposal seems like a usable solution for your problem while > > complying > > to the device tree principle. So instead of overriding the default > > brcmfmac should modify it when dt specifies the "module" property, > > ie: > > > > no "module" in DT: nvram filename = brcm/brcmfmac43362- > > sdio.txt > > "module=ap6210" in DT: nvram filename = brcm/brcmfmac43362- > > ap6210.txt > > > > By the way, the example in the bindings file does not seem to > > specify a > > basename, but path+basename+fileext. > > What is the size of this nvram file? As it's board specific, I wonder > if we can simply include it inside of the DT verbatim. I remember > doing that (in the pre-dtb days, on real open firmware) for the > "spidernet" > ethernet driver. It contains a bit too much info: This is what CubieTruck requires: http://dl.cubieboard.org/public/Cubieboard/benn/firmware/ap6210/nvram_a p6210.txt -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On Wednesday, June 29, 2016 8:51:44 PM CEST Arend Van Spriel wrote: > > Typical wifi devices will have some sort of non volatile storage > > on board to not only store the ethernet(mac) address, but also > > to contain e.g. info about the antenna gain so that the firmware > > and/or the driver can take the antenna gain into account and ensure > > that they never exceed the maximum allowed broadcast strength. > > > > However on some embedded devices there is no non-volatile storage > > for the wifi (for cost reasons) and instead this configuration info > > (which is board / pcb specific) is loaded in the form of a > > file which contains the contents which would normally be in the > > non-volatile storage. > > > > Since we are dealing with a per-board config-file here, which is > > loaded from the os filesystem we really need to specify a basename > > here as the list of possible boards is endless, so we cannot > > have a lookup table in the driver. > > As Jonas mentioned the general principle of device tree is to be > agnostic with regards to OS and/or driver as you undoubtedly know. His > proposal seems like a usable solution for your problem while complying > to the device tree principle. So instead of overriding the default > brcmfmac should modify it when dt specifies the "module" property, ie: > > no "module" in DT: nvram filename = brcm/brcmfmac43362-sdio.txt > "module=ap6210" in DT: nvram filename = brcm/brcmfmac43362-ap6210.txt > > By the way, the example in the bindings file does not seem to specify a > basename, but path+basename+fileext. What is the size of this nvram file? As it's board specific, I wonder if we can simply include it inside of the DT verbatim. I remember doing that (in the pre-dtb days, on real open firmware) for the "spidernet" ethernet driver. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On 29-6-2016 20:51, Arend Van Spriel wrote: > On 29-6-2016 20:01, Hans de Goede wrote: >> Hi, >> >> On 29-06-16 19:00, Kalle Valo wrote: >>> Hans de Goede writes: >>> Hi, On 29-06-16 16:42, Jonas Gorski wrote: > Hi, > > On 29 June 2016 at 16:04, 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. > > Directly defining a filename doesn't seem like a good OS-agnostic > approach. Maybe an alternative would be to add a model-property to the > nodes (this is allowed) and make brcmfmac to request > "FWFILENAME-" as firmware if set? That would leave it to the OS > on how the filename is set. It only defines the base-filename, not the entire path, how / where this file is searched for / loaded-from is then left up to the os >>> >>> It's still a bad idea. The filename, including the path, should be >>> created in the driver. Can't you provide chipname (or similar) via >>> device tree and then the driver can choose what image to use? >> >> No, the driver already does that, but this is not ... >> >>> Can you tell more about the naming the firmware image, how does it work >>> exactly? >> >> About firmware, this is about the nvram file which is board specific, >> rather then chip specific. > > The nvram filename is determined pretty much the same as the firmware > filename, but indeed the nvram data is board specific. This is main > reason why we do not submit nvram files to linux-firmware, because we > simply do not have those files. > >> Typical wifi devices will have some sort of non volatile storage >> on board to not only store the ethernet(mac) address, but also >> to contain e.g. info about the antenna gain so that the firmware >> and/or the driver can take the antenna gain into account and ensure >> that they never exceed the maximum allowed broadcast strength. >> >> However on some embedded devices there is no non-volatile storage >> for the wifi (for cost reasons) and instead this configuration info >> (which is board / pcb specific) is loaded in the form of a >> file which contains the contents which would normally be in the >> non-volatile storage. >> >> Since we are dealing with a per-board config-file here, which is >> loaded from the os filesystem we really need to specify a basename >> here as the list of possible boards is endless, so we cannot >> have a lookup table in the driver. As a note: For BT Marcel was playing with the idea of having a lookup table on the file system which would be loaded by the driver. > As Jonas mentioned the general principle of device tree is to be > agnostic with regards to OS and/or driver as you undoubtedly know. His > proposal seems like a usable solution for your problem while complying > to the device tree principle. So instead of overriding the default > brcmfmac should modify it when dt specifies the "module" property, ie: > > no "module" in DT:nvram filename = brcm/brcmfmac43362-sdio.txt > "module=ap6210" in DT:nvram filename = brcm/brcmfmac43362-ap6210.txt > > By the way, the example in the bindings file does not seem to specify a > basename, but path+basename+fileext. Hans, Another btw: Kalle has taken over maintainer job from John. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
On 29-6-2016 20:01, Hans de Goede wrote: > Hi, > > On 29-06-16 19:00, Kalle Valo wrote: >> Hans de Goede writes: >> >>> Hi, >>> >>> On 29-06-16 16:42, Jonas Gorski wrote: Hi, On 29 June 2016 at 16:04, 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. Directly defining a filename doesn't seem like a good OS-agnostic approach. Maybe an alternative would be to add a model-property to the nodes (this is allowed) and make brcmfmac to request "FWFILENAME-" as firmware if set? That would leave it to the OS on how the filename is set. >>> >>> It only defines the base-filename, not the entire path, how / where >>> this file is searched for / loaded-from is then left up to the os >> >> It's still a bad idea. The filename, including the path, should be >> created in the driver. Can't you provide chipname (or similar) via >> device tree and then the driver can choose what image to use? > > No, the driver already does that, but this is not ... > >> Can you tell more about the naming the firmware image, how does it work >> exactly? > > About firmware, this is about the nvram file which is board specific, > rather then chip specific. The nvram filename is determined pretty much the same as the firmware filename, but indeed the nvram data is board specific. This is main reason why we do not submit nvram files to linux-firmware, because we simply do not have those files. > Typical wifi devices will have some sort of non volatile storage > on board to not only store the ethernet(mac) address, but also > to contain e.g. info about the antenna gain so that the firmware > and/or the driver can take the antenna gain into account and ensure > that they never exceed the maximum allowed broadcast strength. > > However on some embedded devices there is no non-volatile storage > for the wifi (for cost reasons) and instead this configuration info > (which is board / pcb specific) is loaded in the form of a > file which contains the contents which would normally be in the > non-volatile storage. > > Since we are dealing with a per-board config-file here, which is > loaded from the os filesystem we really need to specify a basename > here as the list of possible boards is endless, so we cannot > have a lookup table in the driver. As Jonas mentioned the general principle of device tree is to be agnostic with regards to OS and/or driver as you undoubtedly know. His proposal seems like a usable solution for your problem while complying to the device tree principle. So instead of overriding the default brcmfmac should modify it when dt specifies the "module" property, ie: no "module" in DT: nvram filename = brcm/brcmfmac43362-sdio.txt "module=ap6210" in DT: nvram filename = brcm/brcmfmac43362-ap6210.txt By the way, the example in the bindings file does not seem to specify a basename, but path+basename+fileext. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property
Hi, On 29-06-16 19:00, Kalle Valo wrote: Hans de Goede writes: Hi, On 29-06-16 16:42, Jonas Gorski wrote: Hi, On 29 June 2016 at 16:04, 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. Directly defining a filename doesn't seem like a good OS-agnostic approach. Maybe an alternative would be to add a model-property to the nodes (this is allowed) and make brcmfmac to request "FWFILENAME-" as firmware if set? That would leave it to the OS on how the filename is set. It only defines the base-filename, not the entire path, how / where this file is searched for / loaded-from is then left up to the os It's still a bad idea. The filename, including the path, should be created in the driver. Can't you provide chipname (or similar) via device tree and then the driver can choose what image to use? No, the driver already does that, but this is not ... Can you tell more about the naming the firmware image, how does it work exactly? About firmware, this is about the nvram file which is board specific, rather then chip specific. Typical wifi devices will have some sort of non volatile storage on board to not only store the ethernet(mac) address, but also to contain e.g. info about the antenna gain so that the firmware and/or the driver can take the antenna gain into account and ensure that they never exceed the maximum allowed broadcast strength. However on some embedded devices there is no non-volatile storage for the wifi (for cost reasons) and instead this configuration info (which is board / pcb specific) is loaded in the form of a file which contains the contents which would normally be in the non-volatile storage. Since we are dealing with a per-board config-file here, which is loaded from the os filesystem we really need to specify a basename here as the list of possible boards is endless, so we cannot have a lookup table in the driver. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html