| Message ID | 20241120095428.1122935-2-quic_chejiang@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add qcom,product-variant properties in Qualcomm | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
This is an automated email and please do not reply to this email. Dear Submitter, Thank you for submitting the patches to the linux bluetooth mailing list. While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository. ----- Output ----- error: patch failed: arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi:856 error: arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi: patch does not apply hint: Use 'git am --show-current-patch' to see the failed patch Please resolve the issue and submit the patches again. --- Regards, Linux Bluetooth
On Wed, Nov 20, 2024 at 05:54:25PM +0800, Cheng Jiang wrote: > Several Qualcomm projects will use the same Bluetooth chip, each > focusing on different features. For instance, consumer projects > prioritize the A2DP SRC feature, while IoT projects focus on the A2DP > SINK feature, which may have more optimizations for coexistence when > acting as a SINK. Due to the patch size, it is not feasible to include > all features in a single firmware. > > Therefore, the 'product-variant' devicetree property is used to provide > product information for the Bluetooth driver to load the appropriate > firmware. > > If this property is not defined, the default firmware will be loaded, > ensuring there are no backward compatibility issues with older > devicetrees. > > The product-variant defines like this: > 0 - 15 (16 bits) are product line specific definitions > 16 - 23 (8 bits) are for the product line. > 24 - 31 (8 bits) are reserved for future use, 0 currently Please use text strings instead of encoding this information into random integers and then using just 3 bits out of 32. > > |---------------------------------------------------------------------| > | 32 Bits | > |---------------------------------------------------------------------| > | 31 - 24 (bits) | 23 - 16 (bits) | 15 - 0 (16 bits) | > |---------------------------------------------------------------------| > | Reserved | 0: default | 0: default | > | | 1: CE | | > | | 2: IoT | | > | | 3: Auto | | > | | 4: Reserved | | > |---------------------------------------------------------------------| > > Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com> > --- > .../bindings/net/bluetooth/qualcomm-bluetooth.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml > index 7bb68311c609..9019fe7bcdc6 100644 > --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml > +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml > @@ -110,6 +110,12 @@ properties: > description: > boot firmware is incorrectly passing the address in big-endian order > > + qcom,product-variant: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + specify the product information for driver to load the appropriate firmware DT describes hardware. Is this a hardware property? > + > + > required: > - compatible > > -- > 2.25.1 >
On 20/11/2024 10:54, Cheng Jiang wrote: > Several Qualcomm projects will use the same Bluetooth chip, each > focusing on different features. For instance, consumer projects > prioritize the A2DP SRC feature, while IoT projects focus on the A2DP > SINK feature, which may have more optimizations for coexistence when > acting as a SINK. Due to the patch size, it is not feasible to include > all features in a single firmware. > > Therefore, the 'product-variant' devicetree property is used to provide > product information for the Bluetooth driver to load the appropriate > firmware. > > If this property is not defined, the default firmware will be loaded, > ensuring there are no backward compatibility issues with older > devicetrees. > > The product-variant defines like this: > 0 - 15 (16 bits) are product line specific definitions > 16 - 23 (8 bits) are for the product line. > 24 - 31 (8 bits) are reserved for future use, 0 currently > > |---------------------------------------------------------------------| > | 32 Bits | > |---------------------------------------------------------------------| > | 31 - 24 (bits) | 23 - 16 (bits) | 15 - 0 (16 bits) | > |---------------------------------------------------------------------| > | Reserved | 0: default | 0: default | > | | 1: CE | | > | | 2: IoT | | > | | 3: Auto | | > | | 4: Reserved | | > |---------------------------------------------------------------------| > > Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com> > --- > .../bindings/net/bluetooth/qualcomm-bluetooth.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml > index 7bb68311c609..9019fe7bcdc6 100644 > --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml > +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml > @@ -110,6 +110,12 @@ properties: > description: > boot firmware is incorrectly passing the address in big-endian order > > + qcom,product-variant: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + specify the product information for driver to load the appropriate firmware Nah, you have firmware-name for this. > + > + No clue why two blank lines... Best regards, Krzysztof
Hi Dmitry, On 11/20/2024 6:43 PM, Dmitry Baryshkov wrote: > On Wed, Nov 20, 2024 at 05:54:25PM +0800, Cheng Jiang wrote: >> Several Qualcomm projects will use the same Bluetooth chip, each >> focusing on different features. For instance, consumer projects >> prioritize the A2DP SRC feature, while IoT projects focus on the A2DP >> SINK feature, which may have more optimizations for coexistence when >> acting as a SINK. Due to the patch size, it is not feasible to include >> all features in a single firmware. >> >> Therefore, the 'product-variant' devicetree property is used to provide >> product information for the Bluetooth driver to load the appropriate >> firmware. >> >> If this property is not defined, the default firmware will be loaded, >> ensuring there are no backward compatibility issues with older >> devicetrees. >> >> The product-variant defines like this: >> 0 - 15 (16 bits) are product line specific definitions >> 16 - 23 (8 bits) are for the product line. >> 24 - 31 (8 bits) are reserved for future use, 0 currently > > Please use text strings instead of encoding this information into random > integers and then using just 3 bits out of 32. Ack. Originally intended to make it more flexible for future use. It can be text strings for current requirement. > >> >> |---------------------------------------------------------------------| >> | 32 Bits | >> |---------------------------------------------------------------------| >> | 31 - 24 (bits) | 23 - 16 (bits) | 15 - 0 (16 bits) | >> |---------------------------------------------------------------------| >> | Reserved | 0: default | 0: default | >> | | 1: CE | | >> | | 2: IoT | | >> | | 3: Auto | | >> | | 4: Reserved | | >> |---------------------------------------------------------------------| >> >> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com> >> --- >> .../bindings/net/bluetooth/qualcomm-bluetooth.yaml | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >> index 7bb68311c609..9019fe7bcdc6 100644 >> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >> @@ -110,6 +110,12 @@ properties: >> description: >> boot firmware is incorrectly passing the address in big-endian order >> >> + qcom,product-variant: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + specify the product information for driver to load the appropriate firmware > > DT describes hardware. Is this a hardware property? It has been added to identify the firmware image for the platform. The driver parses it, and then the rampatch is selected from a specify directory. Currently, there is a 'firmware-name' parameter, but it is only used to specify the NVM (config) file. We also need to specify the rampatch (TLV file). Can we re-use the "firmware-name"? add two segments like the following? firmware-name = "rampatch_xx.tlv", "nvm_xx.bin"; Or add a new property to specify the rampatch file? rampatch-name = "rampatch_xx.tlv"; > >> + >> + >> required: >> - compatible >> >> -- >> 2.25.1 >> >
Hi Krzysztof, On 11/21/2024 12:47 AM, Krzysztof Kozlowski wrote: > On 20/11/2024 10:54, Cheng Jiang wrote: >> Several Qualcomm projects will use the same Bluetooth chip, each >> focusing on different features. For instance, consumer projects >> prioritize the A2DP SRC feature, while IoT projects focus on the A2DP >> SINK feature, which may have more optimizations for coexistence when >> acting as a SINK. Due to the patch size, it is not feasible to include >> all features in a single firmware. >> >> Therefore, the 'product-variant' devicetree property is used to provide >> product information for the Bluetooth driver to load the appropriate >> firmware. >> >> If this property is not defined, the default firmware will be loaded, >> ensuring there are no backward compatibility issues with older >> devicetrees. >> >> The product-variant defines like this: >> 0 - 15 (16 bits) are product line specific definitions >> 16 - 23 (8 bits) are for the product line. >> 24 - 31 (8 bits) are reserved for future use, 0 currently >> >> |---------------------------------------------------------------------| >> | 32 Bits | >> |---------------------------------------------------------------------| >> | 31 - 24 (bits) | 23 - 16 (bits) | 15 - 0 (16 bits) | >> |---------------------------------------------------------------------| >> | Reserved | 0: default | 0: default | >> | | 1: CE | | >> | | 2: IoT | | >> | | 3: Auto | | >> | | 4: Reserved | | >> |---------------------------------------------------------------------| >> >> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com> >> --- >> .../bindings/net/bluetooth/qualcomm-bluetooth.yaml | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >> index 7bb68311c609..9019fe7bcdc6 100644 >> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >> @@ -110,6 +110,12 @@ properties: >> description: >> boot firmware is incorrectly passing the address in big-endian order >> >> + qcom,product-variant: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + specify the product information for driver to load the appropriate firmware > > Nah, you have firmware-name for this. > Currently "firmware-name" is used to specifythe nvm (config) file only, we also need to specify the rampatch file (TLV). Can we re-use the "firmware-name"? add two segments like the following? firmware-name = "rampatch_xx.tlv", "nvm_xx.bin"; Or add a new property to specify the rampatch file? rampatch-name = "rampatch_xx.tlv"; Thanks! > >> + >> + > No clue why two blank lines... > > Best regards, > Krzysztof
On Thu, 21 Nov 2024 at 06:02, Cheng Jiang <quic_chejiang@quicinc.com> wrote: > > Hi Dmitry, > > On 11/20/2024 6:43 PM, Dmitry Baryshkov wrote: > > On Wed, Nov 20, 2024 at 05:54:25PM +0800, Cheng Jiang wrote: > >> Several Qualcomm projects will use the same Bluetooth chip, each > >> focusing on different features. For instance, consumer projects > >> prioritize the A2DP SRC feature, while IoT projects focus on the A2DP > >> SINK feature, which may have more optimizations for coexistence when > >> acting as a SINK. Due to the patch size, it is not feasible to include > >> all features in a single firmware. > >> > >> Therefore, the 'product-variant' devicetree property is used to provide > >> product information for the Bluetooth driver to load the appropriate > >> firmware. > >> > >> If this property is not defined, the default firmware will be loaded, > >> ensuring there are no backward compatibility issues with older > >> devicetrees. > >> > >> The product-variant defines like this: > >> 0 - 15 (16 bits) are product line specific definitions > >> 16 - 23 (8 bits) are for the product line. > >> 24 - 31 (8 bits) are reserved for future use, 0 currently > > > > Please use text strings instead of encoding this information into random > > integers and then using just 3 bits out of 32. > Ack. Originally intended to make it more flexible for future use. It can be > text strings for current requirement. No, fixed-format data isn't flexible. Fine-grained properties are. Please define exactly what is necessary rather than leaving empty holes "for future expansion".= > > > >> > >> |---------------------------------------------------------------------| > >> | 32 Bits | > >> |---------------------------------------------------------------------| > >> | 31 - 24 (bits) | 23 - 16 (bits) | 15 - 0 (16 bits) | > >> |---------------------------------------------------------------------| > >> | Reserved | 0: default | 0: default | > >> | | 1: CE | | > >> | | 2: IoT | | > >> | | 3: Auto | | > >> | | 4: Reserved | | > >> |---------------------------------------------------------------------| > >> > >> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com> > >> --- > >> .../bindings/net/bluetooth/qualcomm-bluetooth.yaml | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml > >> index 7bb68311c609..9019fe7bcdc6 100644 > >> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml > >> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml > >> @@ -110,6 +110,12 @@ properties: > >> description: > >> boot firmware is incorrectly passing the address in big-endian order > >> > >> + qcom,product-variant: > >> + $ref: /schemas/types.yaml#/definitions/uint32 > >> + description: > >> + specify the product information for driver to load the appropriate firmware > > > > DT describes hardware. Is this a hardware property? > > It has been added to identify the firmware image for the platform. The driver > parses it, and then the rampatch is selected from a specify directory. Currently, > there is a 'firmware-name' parameter, but it is only used to specify the NVM > (config) file. We also need to specify the rampatch (TLV file). > > > Can we re-use the "firmware-name"? add two segments like the following? > firmware-name = "rampatch_xx.tlv", "nvm_xx.bin"; I think this is the better solution > > Or add a new property to specify the rampatch file? > rampatch-name = "rampatch_xx.tlv"; > > > > >> + > >> + > >> required: > >> - compatible > >> > >> -- > >> 2.25.1 > >> > > >
Hi Dmitry, On 11/21/2024 12:38 PM, Dmitry Baryshkov wrote: > On Thu, 21 Nov 2024 at 06:02, Cheng Jiang <quic_chejiang@quicinc.com> wrote: >> >> Hi Dmitry, >> >> On 11/20/2024 6:43 PM, Dmitry Baryshkov wrote: >>> On Wed, Nov 20, 2024 at 05:54:25PM +0800, Cheng Jiang wrote: >>>> Several Qualcomm projects will use the same Bluetooth chip, each >>>> focusing on different features. For instance, consumer projects >>>> prioritize the A2DP SRC feature, while IoT projects focus on the A2DP >>>> SINK feature, which may have more optimizations for coexistence when >>>> acting as a SINK. Due to the patch size, it is not feasible to include >>>> all features in a single firmware. >>>> >>>> Therefore, the 'product-variant' devicetree property is used to provide >>>> product information for the Bluetooth driver to load the appropriate >>>> firmware. >>>> >>>> If this property is not defined, the default firmware will be loaded, >>>> ensuring there are no backward compatibility issues with older >>>> devicetrees. >>>> >>>> The product-variant defines like this: >>>> 0 - 15 (16 bits) are product line specific definitions >>>> 16 - 23 (8 bits) are for the product line. >>>> 24 - 31 (8 bits) are reserved for future use, 0 currently >>> >>> Please use text strings instead of encoding this information into random >>> integers and then using just 3 bits out of 32. >> Ack. Originally intended to make it more flexible for future use. It can be >> text strings for current requirement. > > No, fixed-format data isn't flexible. Fine-grained properties are. > Please define exactly what is necessary rather than leaving empty > holes "for future expansion".= Got it. Thank you for the guidance. > >>> >>>> >>>> |---------------------------------------------------------------------| >>>> | 32 Bits | >>>> |---------------------------------------------------------------------| >>>> | 31 - 24 (bits) | 23 - 16 (bits) | 15 - 0 (16 bits) | >>>> |---------------------------------------------------------------------| >>>> | Reserved | 0: default | 0: default | >>>> | | 1: CE | | >>>> | | 2: IoT | | >>>> | | 3: Auto | | >>>> | | 4: Reserved | | >>>> |---------------------------------------------------------------------| >>>> >>>> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com> >>>> --- >>>> .../bindings/net/bluetooth/qualcomm-bluetooth.yaml | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >>>> index 7bb68311c609..9019fe7bcdc6 100644 >>>> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >>>> @@ -110,6 +110,12 @@ properties: >>>> description: >>>> boot firmware is incorrectly passing the address in big-endian order >>>> >>>> + qcom,product-variant: >>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>> + description: >>>> + specify the product information for driver to load the appropriate firmware >>> >>> DT describes hardware. Is this a hardware property? >> >> It has been added to identify the firmware image for the platform. The driver >> parses it, and then the rampatch is selected from a specify directory. Currently, >> there is a 'firmware-name' parameter, but it is only used to specify the NVM >> (config) file. We also need to specify the rampatch (TLV file). >> >> >> Can we re-use the "firmware-name"? add two segments like the following? >> firmware-name = "rampatch_xx.tlv", "nvm_xx.bin"; > > I think this is the better solution Ack. Will submit a new change based on this. > >> >> Or add a new property to specify the rampatch file? >> rampatch-name = "rampatch_xx.tlv"; >> >>> >>>> + >>>> + >>>> required: >>>> - compatible >>>> >>>> -- >>>> 2.25.1 >>>> >>> >> > >
On 21/11/2024 05:06, Cheng Jiang wrote: > Hi Krzysztof, > > On 11/21/2024 12:47 AM, Krzysztof Kozlowski wrote: >> On 20/11/2024 10:54, Cheng Jiang wrote: >>> Several Qualcomm projects will use the same Bluetooth chip, each >>> focusing on different features. For instance, consumer projects >>> prioritize the A2DP SRC feature, while IoT projects focus on the A2DP >>> SINK feature, which may have more optimizations for coexistence when >>> acting as a SINK. Due to the patch size, it is not feasible to include >>> all features in a single firmware. >>> >>> Therefore, the 'product-variant' devicetree property is used to provide >>> product information for the Bluetooth driver to load the appropriate >>> firmware. >>> >>> If this property is not defined, the default firmware will be loaded, >>> ensuring there are no backward compatibility issues with older >>> devicetrees. >>> >>> The product-variant defines like this: >>> 0 - 15 (16 bits) are product line specific definitions >>> 16 - 23 (8 bits) are for the product line. >>> 24 - 31 (8 bits) are reserved for future use, 0 currently >>> >>> |---------------------------------------------------------------------| >>> | 32 Bits | >>> |---------------------------------------------------------------------| >>> | 31 - 24 (bits) | 23 - 16 (bits) | 15 - 0 (16 bits) | >>> |---------------------------------------------------------------------| >>> | Reserved | 0: default | 0: default | >>> | | 1: CE | | >>> | | 2: IoT | | >>> | | 3: Auto | | >>> | | 4: Reserved | | >>> |---------------------------------------------------------------------| >>> >>> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com> >>> --- >>> .../bindings/net/bluetooth/qualcomm-bluetooth.yaml | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >>> index 7bb68311c609..9019fe7bcdc6 100644 >>> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >>> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >>> @@ -110,6 +110,12 @@ properties: >>> description: >>> boot firmware is incorrectly passing the address in big-endian order >>> >>> + qcom,product-variant: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: >>> + specify the product information for driver to load the appropriate firmware >> >> Nah, you have firmware-name for this. >> > Currently "firmware-name" is used to specifythe nvm (config) file only, > we also need to specify the rampatch file (TLV). > > Can we re-use the "firmware-name"? add two segments like the following? > firmware-name = "rampatch_xx.tlv", "nvm_xx.bin"; > > Or add a new property to specify the rampatch file? > rampatch-name = "rampatch_xx.tlv"; You can grow the property, it's a list. Order of items in the list must be fixed (specific), though. See other Qualcomm remoteproc PAS loaders which already use two entries. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml index 7bb68311c609..9019fe7bcdc6 100644 --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml @@ -110,6 +110,12 @@ properties: description: boot firmware is incorrectly passing the address in big-endian order + qcom,product-variant: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + specify the product information for driver to load the appropriate firmware + + required: - compatible
Several Qualcomm projects will use the same Bluetooth chip, each focusing on different features. For instance, consumer projects prioritize the A2DP SRC feature, while IoT projects focus on the A2DP SINK feature, which may have more optimizations for coexistence when acting as a SINK. Due to the patch size, it is not feasible to include all features in a single firmware. Therefore, the 'product-variant' devicetree property is used to provide product information for the Bluetooth driver to load the appropriate firmware. If this property is not defined, the default firmware will be loaded, ensuring there are no backward compatibility issues with older devicetrees. The product-variant defines like this: 0 - 15 (16 bits) are product line specific definitions 16 - 23 (8 bits) are for the product line. 24 - 31 (8 bits) are reserved for future use, 0 currently |---------------------------------------------------------------------| | 32 Bits | |---------------------------------------------------------------------| | 31 - 24 (bits) | 23 - 16 (bits) | 15 - 0 (16 bits) | |---------------------------------------------------------------------| | Reserved | 0: default | 0: default | | | 1: CE | | | | 2: IoT | | | | 3: Auto | | | | 4: Reserved | | |---------------------------------------------------------------------| Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com> --- .../bindings/net/bluetooth/qualcomm-bluetooth.yaml | 6 ++++++ 1 file changed, 6 insertions(+)