| Message ID | 1554368908-22017-2-git-send-email-rjliao@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Bluetooth: hci_qca: Load customized NVM based on the device property | expand |
+robh On 04/04/2019 11:08, Rocky Liao wrote: > This patchs patch adds an optional device property nvm-postfix to allow the > driver to load customized nvm file based on this property While text /before/ is indeed called a "prefix", text /after/ is not a "postfix", but a "suffix". > Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > index 824c0e2..70cda4b 100644 > --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > @@ -16,6 +16,7 @@ Optional properties for compatible string qcom,qca6174-bt: > > - enable-gpios: gpio specifier used to enable chip > - clocks: clock provided to the controller (SUSCLK_32KHZ) > + - nvm-postfix: nvm file postfix to load customized nvm file The device tree is supposed to describe hardware. The name of which file to load can hardly be considered part of the HW. Possible solutions: 1) derive the file name from the compatible string 2) pass the name as a module parameter 3) something else > @@ -39,6 +40,7 @@ serial@7570000 { > > enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; > clocks = <&divclk4>; > + nvm-postfix = "i2s"; > }; > }; If one provides the entire suffix, including the underscore, then you can simplify the code: snprintf(config.fwname, sizeof(config.fwname), "qca/nvm_%08x%s.bin", soc_ver, suffix ? suffix : ""); Regards.
On 2019-04-04 20:32, Marc Gonzalez wrote: > +robh > > On 04/04/2019 11:08, Rocky Liao wrote: > >> This patchs patch adds an optional device property nvm-postfix to >> allow the >> driver to load customized nvm file based on this property > > While text /before/ is indeed called a "prefix", text /after/ is not a > "postfix", > but a "suffix". > > >> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> index 824c0e2..70cda4b 100644 >> --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> @@ -16,6 +16,7 @@ Optional properties for compatible string >> qcom,qca6174-bt: >> >> - enable-gpios: gpio specifier used to enable chip >> - clocks: clock provided to the controller (SUSCLK_32KHZ) >> + - nvm-postfix: nvm file postfix to load customized nvm file > > The device tree is supposed to describe hardware. > > The name of which file to load can hardly be considered part of the HW. > > Possible solutions: > 1) derive the file name from the compatible string > 2) pass the name as a module parameter > 3) something else > > >> @@ -39,6 +40,7 @@ serial@7570000 { >> >> enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; >> clocks = <&divclk4>; >> + nvm-postfix = "i2s"; >> }; >> }; > > If one provides the entire suffix, including the underscore, then you > can > simplify the code: > > snprintf(config.fwname, sizeof(config.fwname), "qca/nvm_%08x%s.bin", > soc_ver, suffix ? suffix : ""); > > Regards . Hi Marc, The major purpose for that property is about the BT audio bus type, can it be considered as part of the HW? If yes maybe we can use a property name "audio-bus" to reflect that. If not then I will adopt the solution 1 to add a new compatible string "{ .compatible = "qcom,qca6174-bt-i2s" }" and load specific nvm for this compatible string, please feel free to let me know if any other concerns.
On Thu, Apr 4, 2019 at 7:32 AM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote: > > +robh > > On 04/04/2019 11:08, Rocky Liao wrote: > > > This patchs patch adds an optional device property nvm-postfix to allow the > > driver to load customized nvm file based on this property > > While text /before/ is indeed called a "prefix", text /after/ is not a "postfix", > but a "suffix". > > > > Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > > index 824c0e2..70cda4b 100644 > > --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > > +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > > @@ -16,6 +16,7 @@ Optional properties for compatible string qcom,qca6174-bt: > > > > - enable-gpios: gpio specifier used to enable chip > > - clocks: clock provided to the controller (SUSCLK_32KHZ) > > + - nvm-postfix: nvm file postfix to load customized nvm file > > The device tree is supposed to describe hardware. > > The name of which file to load can hardly be considered part of the HW. > > Possible solutions: > 1) derive the file name from the compatible string > 2) pass the name as a module parameter > 3) something else Or use the 'firmware-name' property to define the full firmware filename. Rob
On Tue, Apr 9, 2019 at 5:15 AM Rocky Liao <rjliao@codeaurora.org> wrote: > > On 2019-04-04 20:32, Marc Gonzalez wrote: > > +robh > > > > On 04/04/2019 11:08, Rocky Liao wrote: > > > >> This patchs patch adds an optional device property nvm-postfix to > >> allow the > >> driver to load customized nvm file based on this property > > > > While text /before/ is indeed called a "prefix", text /after/ is not a > > "postfix", > > but a "suffix". > > > > > >> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > >> b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > >> index 824c0e2..70cda4b 100644 > >> --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > >> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > >> @@ -16,6 +16,7 @@ Optional properties for compatible string > >> qcom,qca6174-bt: > >> > >> - enable-gpios: gpio specifier used to enable chip > >> - clocks: clock provided to the controller (SUSCLK_32KHZ) > >> + - nvm-postfix: nvm file postfix to load customized nvm file > > > > The device tree is supposed to describe hardware. > > > > The name of which file to load can hardly be considered part of the HW. > > > > Possible solutions: > > 1) derive the file name from the compatible string > > 2) pass the name as a module parameter > > 3) something else > > > > > >> @@ -39,6 +40,7 @@ serial@7570000 { > >> > >> enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; > >> clocks = <&divclk4>; > >> + nvm-postfix = "i2s"; > >> }; > >> }; > > > > If one provides the entire suffix, including the underscore, then you > > can > > simplify the code: > > > > snprintf(config.fwname, sizeof(config.fwname), "qca/nvm_%08x%s.bin", > > soc_ver, suffix ? suffix : ""); > > > > Regards > . > Hi Marc, > > The major purpose for that property is about the BT audio bus type, can > it be considered as part of the HW? If yes maybe we can use a property > name "audio-bus" to reflect that. > > If not then I will adopt the solution 1 to add a new compatible string > "{ .compatible = "qcom,qca6174-bt-i2s" }" and load specific nvm for this > compatible string, please feel free to let me know if any other > concerns. I don't think the suggestion was to add the nvm string to the compatible, but rather compatible strings serve as a map key. Having board specific firmware files for wifi/bt is pretty common, but parameters for 'i2s' is a bit strange. So a better explanation of what parameters this contains would help. How/when does it vary, for example? Also, if it is only a handful of parameters, making them DT properties is preferred. Rob
On 2019-04-09 22:03, Rob Herring wrote: > On Tue, Apr 9, 2019 at 5:15 AM Rocky Liao <rjliao@codeaurora.org> > wrote: >> >> On 2019-04-04 20:32, Marc Gonzalez wrote: >> > +robh >> > >> > On 04/04/2019 11:08, Rocky Liao wrote: >> > >> >> This patchs patch adds an optional device property nvm-postfix to >> >> allow the >> >> driver to load customized nvm file based on this property >> > >> > While text /before/ is indeed called a "prefix", text /after/ is not a >> > "postfix", >> > but a "suffix". >> > >> > >> >> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++ >> >> 1 file changed, 2 insertions(+) >> >> >> >> diff --git >> >> a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> >> b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> >> index 824c0e2..70cda4b 100644 >> >> --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> >> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> >> @@ -16,6 +16,7 @@ Optional properties for compatible string >> >> qcom,qca6174-bt: >> >> >> >> - enable-gpios: gpio specifier used to enable chip >> >> - clocks: clock provided to the controller (SUSCLK_32KHZ) >> >> + - nvm-postfix: nvm file postfix to load customized nvm file >> > >> > The device tree is supposed to describe hardware. >> > >> > The name of which file to load can hardly be considered part of the HW. >> > >> > Possible solutions: >> > 1) derive the file name from the compatible string >> > 2) pass the name as a module parameter >> > 3) something else >> > >> > >> >> @@ -39,6 +40,7 @@ serial@7570000 { >> >> >> >> enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; >> >> clocks = <&divclk4>; >> >> + nvm-postfix = "i2s"; >> >> }; >> >> }; >> > >> > If one provides the entire suffix, including the underscore, then you >> > can >> > simplify the code: >> > >> > snprintf(config.fwname, sizeof(config.fwname), "qca/nvm_%08x%s.bin", >> > soc_ver, suffix ? suffix : ""); >> > >> > Regards >> . >> Hi Marc, >> >> The major purpose for that property is about the BT audio bus type, >> can >> it be considered as part of the HW? If yes maybe we can use a property >> name "audio-bus" to reflect that. >> >> If not then I will adopt the solution 1 to add a new compatible string >> "{ .compatible = "qcom,qca6174-bt-i2s" }" and load specific nvm for >> this >> compatible string, please feel free to let me know if any other >> concerns. > > I don't think the suggestion was to add the nvm string to the > compatible, but rather compatible strings serve as a map key. Having > board specific firmware files for wifi/bt is pretty common, but > parameters for 'i2s' is a bit strange. So a better explanation of what > parameters this contains would help. How/when does it vary, for > example? > > Also, if it is only a handful of parameters, making them DT properties > is preferred. > > Rob Ok, I prefer to go with adding a device property "firmware-name" with full firmware name as you suggested in previous comment. -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt index 824c0e2..70cda4b 100644 --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt @@ -16,6 +16,7 @@ Optional properties for compatible string qcom,qca6174-bt: - enable-gpios: gpio specifier used to enable chip - clocks: clock provided to the controller (SUSCLK_32KHZ) + - nvm-postfix: nvm file postfix to load customized nvm file Required properties for compatible string qcom,wcn3990-bt: @@ -39,6 +40,7 @@ serial@7570000 { enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; clocks = <&divclk4>; + nvm-postfix = "i2s"; }; };
This patchs patch adds an optional device property nvm-postfix to allow the driver to load customized nvm file based on this property Signed-off-by: Rocky Liao <rjliao@codeaurora.org> --- Changes in v2: * added the property to the document file Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt * fixed coding style warnings * moved the nvm-postfix to the last entry of the qca_serdev --- Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++ 1 file changed, 2 insertions(+)