On Sat, 30 Nov 2024 at 05:48, Cheng Jiang (IOE) <quic_chejiang@xxxxxxxxxxx> wrote: > > Hi Dmitry, > > On 11/21/2024 12:38 PM, Dmitry Baryshkov wrote: > > On Thu, 21 Nov 2024 at 06:02, Cheng Jiang <quic_chejiang@xxxxxxxxxxx> 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@xxxxxxxxxxx> > >>>> --- > >>>> .../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 > > > How about the following logic for handling 'firmware-name' property: > 1. If there is only one string in firmware-name, it must be the NVM file, which is used > for backward compatibility. > > 2. If there are two strings in firmware-name, the first string is for the rampatch, and > the second string is for the NVM. I'd say, other way around: the first one is always NVM, the second one is rampatch and it is optional. > > 3. Due to variations in RF performance of chips from different foundries, different NVM > configurations are used based on the board ID. If the second string ends with boardid, > the NVM file will be selected according to the board ID. Is there a reason why you can not use the exact firmware name? The firmware name is a part of the board DT file. I assume you know the board ID that has been used for the board. > > > Here are two examples: > > firmware-name = "qca/QCA6698/hpbtfw21.tlv", "qca/QCA6698/hpnv21.bin"; > In this configuration, the driver will use the two files directly. > > > firmware-name = "qca/QCA6698/hpbtfw21.tlv", "qca/QCA6698/hpnv21.boardid"; > In this configuration, the driver will replace boardid with the actual board information. > If the board id is 0x0206, the nvm file name will be qca/QCA6698/hpnv21.b0206 > > >> > >> Or add a new property to specify the rampatch file? > >> rampatch-name = "rampatch_xx.tlv"; > >> > >>> > >>>> + > >>>> + > >>>> required: > >>>> - compatible > >>>> > >>>> -- > >>>> 2.25.1 > >>>> > >>> > >> > > > > > -- With best wishes Dmitry
