Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

['Arend Van Spriel' via linux-sunxi]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Rob Herring]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm, nvram_file_name dt property

[Arnd Bergmann]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

['Arend Van Spriel' via linux-sunxi]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Arnd Bergmann]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

['Arend Van Spriel' via linux-sunxi]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, 

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Arnd Bergmann]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

['Arend Van Spriel' via linux-sunxi]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Arnd Bergmann]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

['Arend van Spriel' via linux-sunxi]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Arnd Bergmann]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

['Arend Van Spriel' via linux-sunxi]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Arnd Bergmann]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

['Arend Van Spriel' via linux-sunxi]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Arnd Bergmann]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

['Arend Van Spriel' via linux-sunxi]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Kalle Valo]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm, nvram_file_name dt property

[Arnd Bergmann]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

['Arend Van Spriel' via linux-sunxi]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Arnd Bergmann]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Hans de Goede]

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

--
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Jonas Gorski]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Hans de Goede]

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

--
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Kalle Valo]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Hans de Goede]

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

--
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Hans de Goede]

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

--
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Hans de Goede]

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

--
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Julian Calaby]

Hi Kalle,

On 30 Jun 2016 6:46 p.m., "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?

Ap6210 is a chip that contains a 43362 and probably some Bluetooth chip
also.

Thanks,

Julian Calaby

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Kalle Valo]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Kalle Valo]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Kalle Valo]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Arnd Bergmann]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Priit Laes]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Arnd Bergmann]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

['Arend Van Spriel' via linux-sunxi]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

['Arend Van Spriel' via linux-sunxi]

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm,nvram_file_name dt property

[Hans de Goede]

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



--
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.