2021-04-11 07:59:01

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-binding: bcm43xx-fmac: add optional brcm,ccode-map

Shawn Guo <[email protected]> writes:

> Add optional brcm,ccode-map property to support translation from ISO3166
> country code to brcmfmac firmware country code and revision.
>
> Signed-off-by: Shawn Guo <[email protected]>
> ---
> .../devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> index cffb2d6876e3..a65ac4384c04 100644
> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> @@ -15,6 +15,12 @@ Optional properties:
> When not specified the device will use in-band SDIO interrupts.
> - interrupt-names : name of the out-of-band interrupt, which must be set
> to "host-wake".
> + - brcm,ccode-map : multiple strings for translating ISO3166 country code to
> + brcmfmac firmware country code and revision. Each string must be in
> + format "AA-BB-num" where:
> + AA is the ISO3166 country code which must be 2 characters.
> + BB is the firmware country code which must be 2 characters.
> + num is the revision number which must fit into signed integer.
>
> Example:
>
> @@ -34,5 +40,6 @@ mmc3: [email protected] {
> interrupt-parent = <&pio>;
> interrupts = <10 8>; /* PH10 / EINT10 */
> interrupt-names = "host-wake";
> + brcm,ccode-map = "JP-JP-78", "US-Q2-86";

The commit log does not answer "Why?". Why this needs to be in device
tree and, for example, not hard coded in the driver?

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


2021-04-12 01:26:32

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-binding: bcm43xx-fmac: add optional brcm,ccode-map

On Sun, Apr 11, 2021 at 10:57:54AM +0300, Kalle Valo wrote:
> Shawn Guo <[email protected]> writes:
>
> > Add optional brcm,ccode-map property to support translation from ISO3166
> > country code to brcmfmac firmware country code and revision.
> >
> > Signed-off-by: Shawn Guo <[email protected]>
> > ---
> > .../devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> > index cffb2d6876e3..a65ac4384c04 100644
> > --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> > @@ -15,6 +15,12 @@ Optional properties:
> > When not specified the device will use in-band SDIO interrupts.
> > - interrupt-names : name of the out-of-band interrupt, which must be set
> > to "host-wake".
> > + - brcm,ccode-map : multiple strings for translating ISO3166 country code to
> > + brcmfmac firmware country code and revision. Each string must be in
> > + format "AA-BB-num" where:
> > + AA is the ISO3166 country code which must be 2 characters.
> > + BB is the firmware country code which must be 2 characters.
> > + num is the revision number which must fit into signed integer.
> >
> > Example:
> >
> > @@ -34,5 +40,6 @@ mmc3: [email protected] {
> > interrupt-parent = <&pio>;
> > interrupts = <10 8>; /* PH10 / EINT10 */
> > interrupt-names = "host-wake";
> > + brcm,ccode-map = "JP-JP-78", "US-Q2-86";
>
> The commit log does not answer "Why?". Why this needs to be in device
> tree and, for example, not hard coded in the driver?

Thanks for the comment, Kalle. Actually, this is something I need some
input from driver maintainers. I can see this country code mapping
table is chipset specific, and can be hard coded in driver per chip id
and revision. But on the other hand, it makes some sense to have this
table in device tree, as the country code that need to be supported
could be a device specific configuration.

Shawn